From 5fc786e90b814635f20c40588466972b3288239d Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Thu, 30 Mar 2023 14:09:54 -0400 Subject: [PATCH 1/6] Remove simplistic "API" list from endpoints PBENCH-1119 With the REST-ification of the API, only a few of the APIs can still be called using the simplistic `{name: uri}` list, because URI parameters are needed and are no longer at the end of the URI: for example, `/api/v1/datasets/{dataset}/metadata`. The UI now has the `uriTemplate` method to help expand the `"uri"` template objects. Complete the transition by updating the remaining UI references and dropping the `api` object entirely. --- dashboard/src/actions/datasetListActions.js | 5 ++- dashboard/src/actions/overviewActions.js | 24 +++++++---- .../src/actions/tableOfContentActions.js | 4 +- dashboard/src/utils/helper.js | 2 +- .../api/resources/endpoint_configure.py | 40 +++---------------- .../unit/server/test_endpoint_configure.py | 17 -------- 6 files changed, 30 insertions(+), 62 deletions(-) diff --git a/dashboard/src/actions/datasetListActions.js b/dashboard/src/actions/datasetListActions.js index d0c910c0bc..ee4c512a30 100644 --- a/dashboard/src/actions/datasetListActions.js +++ b/dashboard/src/actions/datasetListActions.js @@ -1,13 +1,16 @@ import * as TYPES from "./types"; import API from "../utils/axiosInstance"; +import { uriTemplate } from "utils/helper"; export const fetchPublicDatasets = () => async (dispatch, getState) => { try { dispatch({ type: TYPES.LOADING }); const endpoints = getState().apiEndpoint.endpoints; const response = await API.get( - `${endpoints?.api?.datasets_list}?metadata=dataset.uploaded&access=public` + uriTemplate(endpoints, "datasets_list", {}), + null, + { params: { metadata: "dataset.uploaded", access: "public" } } ); if (response.status === 200 && response.data) { dispatch({ diff --git a/dashboard/src/actions/overviewActions.js b/dashboard/src/actions/overviewActions.js index f3f8051541..07867d2775 100644 --- a/dashboard/src/actions/overviewActions.js +++ b/dashboard/src/actions/overviewActions.js @@ -4,7 +4,7 @@ import * as TYPES from "./types"; import { DANGER, ERROR_MSG } from "assets/constants/toastConstants"; import API from "../utils/axiosInstance"; -import { expandUriTemplate } from "../utils/helper"; +import { uriTemplate } from "../utils/helper"; import { findNoOfDays } from "utils/dateFunctions"; import { showToast } from "./toastActions"; import { clearCachedSession } from "./authActions"; @@ -27,9 +27,13 @@ export const getDatasets = () => async (dispatch, getState) => { params.append("mine", "true"); const endpoints = getState().apiEndpoint.endpoints; - const response = await API.get(endpoints?.api?.datasets_list, { - params: params, - }); + const response = await API.get( + uriTemplate(endpoints, "datasets_list", {}), + null, + { + params: params, + } + ); if (response.status === 200) { if (response?.data?.results?.length > 0) { @@ -127,7 +131,7 @@ export const updateDataset = const method = metaDataActions[actionType]; const endpoints = getState().apiEndpoint.endpoints; - const uri = expandUriTemplate(endpoints, "datasets_metadata", { + const uri = uriTemplate(endpoints, "datasets_metadata", { dataset: dataset.resource_id, }); const response = await API.put(uri, { @@ -176,7 +180,9 @@ export const deleteDataset = (dataset) => async (dispatch, getState) => { dispatch({ type: TYPES.LOADING }); const endpoints = getState().apiEndpoint.endpoints; const response = await API.delete( - `${endpoints.api.datasets}/${dataset.resource_id}` + uriTemplate(endpoints, "datasets_update", { + dataset: dataset.resource_id, + }) ); if (response.status === 200) { const datasets = getState().overview.datasets; @@ -257,7 +263,11 @@ export const publishDataset = const savedRuns = getState().overview.savedRuns; const response = await API.post( - `${endpoints.api.datasets}/${dataset.resource_id}?access=${updateValue}` + uriTemplate(endpoints, `datasets_update`, { + dataset: dataset.resource_id, + }), + null, + { params: { access: updateValue } } ); if (response.status === 200) { const dataIndex = savedRuns.findIndex( diff --git a/dashboard/src/actions/tableOfContentActions.js b/dashboard/src/actions/tableOfContentActions.js index c0dab13550..6136de32e6 100644 --- a/dashboard/src/actions/tableOfContentActions.js +++ b/dashboard/src/actions/tableOfContentActions.js @@ -1,12 +1,12 @@ import * as TYPES from "./types"; import API from "../utils/axiosInstance"; -import { expandUriTemplate } from "../utils/helper"; +import { uriTemplate } from "../utils/helper"; export const fetchTOC = (param, parent, callForSubData) => async (dispatch, getState) => { try { const endpoints = getState().apiEndpoint.endpoints; - const uri = expandUriTemplate(endpoints, "datasets_contents", { + const uri = uriTemplate(endpoints, "datasets_contents", { dataset: param, target: parent, }); diff --git a/dashboard/src/utils/helper.js b/dashboard/src/utils/helper.js index ec0af30161..0aec4e9264 100644 --- a/dashboard/src/utils/helper.js +++ b/dashboard/src/utils/helper.js @@ -13,7 +13,7 @@ export const uid = () => { * @param {Object} args - value for each templated parameter * @return {string} - formatted URI */ -export const expandUriTemplate = (endpoints, name, args) => { +export const uriTemplate = (endpoints, name, args) => { let uri = endpoints.uri[name].template; for (const [key, value] of Object.entries(args)) { uri = uri.replace(`{${key}}`, value); diff --git a/lib/pbench/server/api/resources/endpoint_configure.py b/lib/pbench/server/api/resources/endpoint_configure.py index 21e673f5b3..f44ab2ed13 100644 --- a/lib/pbench/server/api/resources/endpoint_configure.py +++ b/lib/pbench/server/api/resources/endpoint_configure.py @@ -51,17 +51,8 @@ def get(self): including the Pbench dashboard UI. This includes: openid-connect: A JSON object containing the OpenID Connect parameters - required for the web client to use OIDC authentication. + required for the web client to use OIDC authentication. identification: The Pbench server name and version - api: A dict of the server APIs supported; we give a name, which - identifies the service, and the full URI relative to the - configured host name and port (local or remote reverse proxy). - - This is dynamically generated by processing the Flask URI - rules; refer to api/__init__.py for the code which creates - those mappings, or test_endpoint_configure.py for code that - validates the current set (and must be updated when the API - set changes). uri: A dict of server API templates, where each template defines a template URI and a list of typed parameters. @@ -69,28 +60,13 @@ def get(self): prefix (/api/v1/), then replacing the path "/" characters with underscores. - The "api" object contains a key for each API name, where the value is a - simplified URI omitting URI parameters. The client must either know the - required parameters and order, and connect them to the "api" value - separated by slash characters, or refer to the "uri" templates. - - E.g, "/api/v1/controllers/list" yields: - - "controllers_list": "http://host/api/v1/controllers/list" - - while "/api/v1/users/" yields: - - "users": "http://host/api/v1/users" - - For URIs with multiple parameters, or embedded parameters, it may be - easier to work with the template string in the "uri" object. The value - of each API name key in the "uri" object is a minimal "schema" object - defining the template string and parameters for the API. The "uri" - value for the "users" API, for example, will be + The "uri" object defines a template for each API name, defining a set of + URI parameters that must be expanded in the template. For example, the + API to get or modify metadata is: { - "template": "http://host/api/v1/users/{target_username}", - "params": {"target_username": {"type": "string"}} + "template": "http://host/api/v1/datasets/{dataset}/metadata", + "params": {"dataset": {"type": "string"}} } The template can be resolved in Python with: @@ -137,7 +113,6 @@ def get(self): host_value, ) - apis = {} templates = {} # Iterate through the Flask endpoints to add a description for each. @@ -156,7 +131,6 @@ def get(self): for match in matches }, } - url = self.param_template.sub("", url) path = rule.endpoint # We have some URI endpoints that repeat a basic URI pattern. @@ -168,12 +142,10 @@ def get(self): if path not in templates or ( len(template["params"]) > len(templates[path]["params"]) ): - apis[path] = urljoin(host, url) templates[path] = template endpoints = { "identification": f"Pbench server {self.server_config.COMMIT_ID}", - "api": apis, "uri": templates, } diff --git a/lib/pbench/test/unit/server/test_endpoint_configure.py b/lib/pbench/test/unit/server/test_endpoint_configure.py index 96a369faa9..9908c2b258 100644 --- a/lib/pbench/test/unit/server/test_endpoint_configure.py +++ b/lib/pbench/test/unit/server/test_endpoint_configure.py @@ -41,23 +41,6 @@ def check_config(self, client, server_config, host, my_headers={}): uri = urljoin(host, uri_prefix) expected_results = { "identification": f"Pbench server {server_config.COMMIT_ID}", - "api": { - "datasets": f"{uri}/datasets", - "datasets_contents": f"{uri}/datasets/contents", - "datasets_daterange": f"{uri}/datasets/daterange", - "datasets_detail": f"{uri}/datasets/detail", - "datasets_inventory": f"{uri}/datasets/inventory", - "datasets_list": f"{uri}/datasets", - "datasets_mappings": f"{uri}/datasets/mappings", - "datasets_metadata": f"{uri}/datasets/metadata", - "datasets_namespace": f"{uri}/datasets/namespace", - "datasets_search": f"{uri}/datasets/search", - "datasets_values": f"{uri}/datasets/values", - "endpoints": f"{uri}/endpoints", - "server_audit": f"{uri}/server/audit", - "server_settings": f"{uri}/server/settings", - "upload": f"{uri}/upload", - }, "uri": { "datasets": { "template": f"{uri}/datasets/{{dataset}}", From d62c69a238b702745d80956260d64225472e252a Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Thu, 30 Mar 2023 15:03:16 -0400 Subject: [PATCH 2/6] Cleanup and testing --- dashboard/src/actions/datasetListActions.js | 1 - dashboard/src/actions/overviewActions.js | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/dashboard/src/actions/datasetListActions.js b/dashboard/src/actions/datasetListActions.js index ee4c512a30..6cce71cbe7 100644 --- a/dashboard/src/actions/datasetListActions.js +++ b/dashboard/src/actions/datasetListActions.js @@ -9,7 +9,6 @@ export const fetchPublicDatasets = () => async (dispatch, getState) => { const endpoints = getState().apiEndpoint.endpoints; const response = await API.get( uriTemplate(endpoints, "datasets_list", {}), - null, { params: { metadata: "dataset.uploaded", access: "public" } } ); if (response.status === 200 && response.data) { diff --git a/dashboard/src/actions/overviewActions.js b/dashboard/src/actions/overviewActions.js index 07867d2775..6cd9566d65 100644 --- a/dashboard/src/actions/overviewActions.js +++ b/dashboard/src/actions/overviewActions.js @@ -29,9 +29,8 @@ export const getDatasets = () => async (dispatch, getState) => { const endpoints = getState().apiEndpoint.endpoints; const response = await API.get( uriTemplate(endpoints, "datasets_list", {}), - null, { - params: params, + params, } ); @@ -180,7 +179,7 @@ export const deleteDataset = (dataset) => async (dispatch, getState) => { dispatch({ type: TYPES.LOADING }); const endpoints = getState().apiEndpoint.endpoints; const response = await API.delete( - uriTemplate(endpoints, "datasets_update", { + uriTemplate(endpoints, "datasets", { dataset: dataset.resource_id, }) ); @@ -263,7 +262,7 @@ export const publishDataset = const savedRuns = getState().overview.savedRuns; const response = await API.post( - uriTemplate(endpoints, `datasets_update`, { + uriTemplate(endpoints, "datasets", { dataset: dataset.resource_id, }), null, From a7f8f93be75cddc466aa2b770fc3ed603999417f Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Fri, 31 Mar 2023 08:29:48 -0400 Subject: [PATCH 3/6] Oops ... I missed functional tests. :( --- lib/pbench/client/__init__.py | 16 +++++++--------- .../test/functional/server/test_connect.py | 4 ---- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/pbench/client/__init__.py b/lib/pbench/client/__init__.py index fa97d1558d..6537f80cd1 100644 --- a/lib/pbench/client/__init__.py +++ b/lib/pbench/client/__init__.py @@ -120,15 +120,13 @@ def _uri(self, api: API, uri_params: Optional[JSONOBJECT] = None) -> str: Returns: A fully specified URI """ - if not uri_params: - return self.endpoints["api"][api.value] - else: - description = self.endpoints["uri"][api.value] - template = description["template"] - cnt = len(description["params"]) - if cnt != len(uri_params): - raise IncorrectParameterCount(api, cnt, uri_params) - return template.format(**uri_params) + description = self.endpoints["uri"][api.value] + template = description["template"] + cnt = len(description["params"]) + params = uri_params if uri_params else {} + if cnt != len(params): + raise IncorrectParameterCount(api, cnt, params) + return template.format(**params) def get( self, diff --git a/lib/pbench/test/functional/server/test_connect.py b/lib/pbench/test/functional/server/test_connect.py index 01563329a3..9a3202afb7 100644 --- a/lib/pbench/test/functional/server/test_connect.py +++ b/lib/pbench/test/functional/server/test_connect.py @@ -12,19 +12,15 @@ def test_connect(self, server_client: PbenchServerClient): assert server_client.session.headers["Accept"] == "application/json" endpoints = server_client.endpoints assert endpoints - assert "api" in endpoints assert "identification" in endpoints assert "uri" in endpoints # Verify that all expected endpoints are reported - for a in endpoints["api"].keys(): - assert a in expected for a in endpoints["uri"].keys(): assert a in expected # Verify that no unexpected endpoints are reported for e in expected: - assert e in endpoints["api"].keys() assert e in endpoints["uri"].keys() # verify all the required openid-connect fields are present From 05265d25143a543ded2b8f40cdaa3dc7e63afb62 Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Fri, 31 Mar 2023 10:35:33 -0400 Subject: [PATCH 4/6] Like pulling weed roots ... --- lib/pbench/test/unit/client/test_connect.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/pbench/test/unit/client/test_connect.py b/lib/pbench/test/unit/client/test_connect.py index 0b9ef4d09a..6b07b017bd 100644 --- a/lib/pbench/test/unit/client/test_connect.py +++ b/lib/pbench/test/unit/client/test_connect.py @@ -30,7 +30,7 @@ def test_connect(self): url, json={ "identification": "string", - "api": {}, + "uri": {}, "openid": openid_dict, }, @@ -54,7 +54,6 @@ def test_connect(self): # Check that the fake endpoints we returned are captured endpoints = pbench.endpoints assert endpoints - assert endpoints["api"] == {} assert endpoints["identification"] == "string" assert endpoints["uri"] == {} assert endpoints["openid"] == openid_dict From cef1c1599bb7958a43030c258727554a223e8a2d Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Fri, 31 Mar 2023 14:26:57 -0400 Subject: [PATCH 5/6] Default params --- dashboard/src/actions/overviewActions.js | 9 +++------ dashboard/src/utils/helper.js | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/dashboard/src/actions/overviewActions.js b/dashboard/src/actions/overviewActions.js index 6cd9566d65..4ad6a8bb67 100644 --- a/dashboard/src/actions/overviewActions.js +++ b/dashboard/src/actions/overviewActions.js @@ -27,12 +27,9 @@ export const getDatasets = () => async (dispatch, getState) => { params.append("mine", "true"); const endpoints = getState().apiEndpoint.endpoints; - const response = await API.get( - uriTemplate(endpoints, "datasets_list", {}), - { - params, - } - ); + const response = await API.get(uriTemplate(endpoints, "datasets_list"), { + params, + }); if (response.status === 200) { if (response?.data?.results?.length > 0) { diff --git a/dashboard/src/utils/helper.js b/dashboard/src/utils/helper.js index 0aec4e9264..271cee3c8f 100644 --- a/dashboard/src/utils/helper.js +++ b/dashboard/src/utils/helper.js @@ -10,10 +10,10 @@ export const uid = () => { * * @param {Object} endpoints - endpoint object from server * @param {string} name - name of the API to expand - * @param {Object} args - value for each templated parameter + * @param {Object} args - [Optional] value for each templated parameter * @return {string} - formatted URI */ -export const uriTemplate = (endpoints, name, args) => { +export const uriTemplate = (endpoints, name, args = {}) => { let uri = endpoints.uri[name].template; for (const [key, value] of Object.entries(args)) { uri = uri.replace(`{${key}}`, value); From 9ecae5419f09c21d61b43c02477d77136d0fcedf Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Mon, 3 Apr 2023 10:52:54 -0400 Subject: [PATCH 6/6] blackened spices (How'd that happen?) --- lib/pbench/test/unit/client/test_connect.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/pbench/test/unit/client/test_connect.py b/lib/pbench/test/unit/client/test_connect.py index 6b07b017bd..670e2d3b69 100644 --- a/lib/pbench/test/unit/client/test_connect.py +++ b/lib/pbench/test/unit/client/test_connect.py @@ -30,7 +30,6 @@ def test_connect(self): url, json={ "identification": "string", - "uri": {}, "openid": openid_dict, },