From 459c14eca4aced6ce1ae826599765a068cc12317 Mon Sep 17 00:00:00 2001 From: Isla Li Date: Thu, 11 Apr 2024 14:35:45 -0700 Subject: [PATCH 1/5] issue-11: Implement restart, cancel, and status functions and add tests --- onc/+onc/OncDelivery.m | 79 ++++++++++++++++--- onc/+util/is_failed_response.m | 24 ------ onc/tests/globals.m | 8 +- onc/tests/suites/Test07_DataProductDelivery.m | 62 ++++++++++++--- onc/tests/suites/Test08_RealTime.m | 25 +++--- 5 files changed, 140 insertions(+), 58 deletions(-) delete mode 100644 onc/+util/is_failed_response.m diff --git a/onc/+onc/OncDelivery.m b/onc/+onc/OncDelivery.m index 7a0b296..a24cf2a 100644 --- a/onc/+onc/OncDelivery.m +++ b/onc/+onc/OncDelivery.m @@ -76,12 +76,9 @@ fprintf('Requesting data product...\n'); [r, info] = this.doRequest(url, filters); status = info.status; - if not(util.is_failed_response(r, status)) - this.estimatePollPeriod(r); - this.printProductRequest(r); - else - throw(util.prepare_exception(status)); - end + this.estimatePollPeriod(r); + this.printProductRequest(r); + end function [r, status] = runDataProduct(this, dpRequestId, waitComplete) @@ -106,16 +103,18 @@ % run timed run request tic + cancelUrl = url + "?method=cancel&token="+string(this.token)+"&dpRequestId=" + string(dpRequestId); + if waitComplete + fprintf('\nTo cancel this data product, visit url:\n %s\n', cancelUrl); + else + fprintf('\nTo cancel this data product, please execute command ''onc.cancelDataProduct(%d)''\n', dpRequestId) + end flag = 'queued'; while ~strcmp(flag,'complete') && ~strcmp(flag,'cancelled') [response, info] = this.doRequest(url, filters); status = info.status; r.requestCount = r.requestCount + 1; - % guard against failed request - if util.is_failed_response(response, status) - throw(util.prepare_exception(status)); - end % repeat only if waitComplete if waitComplete log.printResponse(response); @@ -140,6 +139,40 @@ r.runIds = [r.runIds, run.dpRunId]; end end + + function response = checkDataProduct(this, dpRequestId) + %% Check the status of a data product + % + % + % checkDataProduct (dpRequestId) + % + % * dpRequestId (int) Request id obtained by requestDataProduct() + % + % Returns: response (struct): status of this data product + % + url = sprintf('%sapi/dataProductDelivery', this.baseUrl); + filters = struct('method', 'status', 'token', this.token, 'dpRequestId', dpRequestId); + response = this.doRequest(url, filters); + end + + function [response, info] = cancelDataProduct(this, dpRequestId) + %% Cancel a running data product + % + % + % cancelDataProduct (dpRequestId) + % + % * dpRequestId (int) Request id obtained by requestDataProduct() + % + % Returns: response (struct): cancel process status and message + % info (struct): cancel process http code and status + % + url = sprintf('%sapi/dataProductDelivery', this.baseUrl); + filters = struct('method', 'cancel', 'token', this.token, 'dpRequestId', dpRequestId); + [response, info] = this.doRequest(url, filters); + if isfield(response, 'status') && strcmp(response.status, 'cancelled') && info.status == 200 + fprintf("Data product with request id %d and run id %d was successfully cancelled\n", dpRequestId, response.dpRunId); + end + end function fileData = downloadDataProduct(this, runId, varargin) %% Download a data product manually with a runId @@ -169,8 +202,32 @@ fileData = this.downloadProductFiles(runId, metadata, maxRetries, overwrite); end end + + function [response, info] = restartDataProduct(this, dpRequestId, waitComplete) + %% Restart a cancelled data product + % + % + % restartDataProduct (dpRequestId, waitComplete) + % + % * dpRequestId (int) Request id obtained by requestDataProduct() + % - waitComplete (optional): wait until dp finish when set to true (default) + % + % Returns: response (struct): restart process status and message + % info (struct): restart process http code and status + % + if ~exist('waitComplete','var'), waitComplete = true; end + url = sprintf('%sapi/dataProductDelivery', this.baseUrl); + filters = struct('method', 'restart', 'token', this.token, 'dpRequestId', dpRequestId); + [response, info] = this.doRequest(url, filters); + if isfield(response, 'status') && (strcmp(response.status, 'data product running') || strcmp(response.status, 'queued')) && info.status == 200 + fprintf("Restarted data product with request id %d and run id %d\n", dpRequestId, response.dpRunId); + end + if waitComplete + [response, info] = this.runDataProduct(dpRequestId, true); + end + end end - + methods (Access = private, Hidden = true) function fileList = downloadProductFiles(this, runId, varargin) %% Download all data product files for provided run id diff --git a/onc/+util/is_failed_response.m b/onc/+util/is_failed_response.m deleted file mode 100644 index 744181c..0000000 --- a/onc/+util/is_failed_response.m +++ /dev/null @@ -1,24 +0,0 @@ -function isFailed = is_failed_response(response, status) - %% Checks if a server response describes a failure - % - % * response: (struct) Response as returned by do_request() - % - status: @TODO - % - % Returns: (Logical) true when the response is a failure - isFailed = false; - - % Fail if HTTP status code is not a 2xx - if exist('status', 'var') - if status < 200 || status > 226 - isFailed = true; - return - end - end - - % Fail if the response is an error description - if isfield(response, "errors") - names = fieldnames(response.errors(1)); - isFailed = ismember("errorCode", names) && ismember("errorMessage", names); - end -end - diff --git a/onc/tests/globals.m b/onc/tests/globals.m index 9293dbe..daa37ac 100644 --- a/onc/tests/globals.m +++ b/onc/tests/globals.m @@ -11,7 +11,7 @@ cd(fileparts(which(mfilename))); end - % grab token from "TOKEN" file + % grab token from "TOKEN" file or get from env f = fopen('TOKEN','r'); if f > 0 token = fgetl(f); @@ -20,8 +20,12 @@ token = getenv('TOKEN'); end + % get environment from ONC_ENV or use QA as default + config.production = getenv('ONC_ENV'); + if isempty(config.production) + config.production = false; + end % Set and save config - config.production = true; config.showInfo = false; config.outPath = 'output'; config.timeout = 60; diff --git a/onc/tests/suites/Test07_DataProductDelivery.m b/onc/tests/suites/Test07_DataProductDelivery.m index ff5073a..6d6f336 100644 --- a/onc/tests/suites/Test07_DataProductDelivery.m +++ b/onc/tests/suites/Test07_DataProductDelivery.m @@ -113,10 +113,40 @@ function testValidResultsOnly(this) end + + %% Testing run method + function testInvalidRequestId(this) + verifyError(this, @() this.onc.runDataProduct(1234567890), 'onc:http400:error127'); + end + + %% Testing download method + function testInvalidRunId(this) + verifyError(this, @() this.onc.downloadDataProduct(1234567890), 'onc:http400:error127'); + end + + %% Testing cancel method + function testCancelWithInvalidRequestId(this) + verifyError(this, @() this.onc.cancelDataProduct(1234567890), 'onc:http400:error127'); + end + + %% Testing status method + function testStatusWithInvalidRequestId(this) + verifyError(this, @() this.onc.checkDataProduct(1234567890), 'onc:http400:error127'); + end + + %% Testing restart method + function testRestartWithInvalidRequestId(this) + verifyError(this, @() this.onc.restartDataProduct(1234567890), 'onc:http400:error127'); + end + %% Integration tests function testValidManual(this) this.updateOncOutPath('output/testValidManual'); requestId = this.onc.requestDataProduct(this.Params).dpRequestId; + statusBeforeDownload = this.onc.checkDataProduct(requestId); + + assertEqual(this, statusBeforeDownload.searchHdrStatus, 'OPEN'); + runId = this.onc.runDataProduct(requestId).runIds(1); data = this.onc.downloadDataProduct(runId); verifyTrue(this, length(data) == 3, ... @@ -133,16 +163,30 @@ function testValidManual(this) verify_files_in_path(this, this.onc.outPath, 3); verify_field_value_type(this, data(1), this.expectedFields); + statusAfterDownload = this.onc.checkDataProduct(requestId); + assertEqual(this, statusAfterDownload.searchHdrStatus, 'COMPLETED'); end - - %% Testing run method - function testInvalidRequestId(this) - verifyError(this, @() this.onc.runDataProduct(1234567890), 'onc:http400:error127'); - end - - %% Testing download method - function testInvalidRunId(this) - verifyError(this, @() this.onc.downloadDataProduct(1234567890), 'onc:http400:error127'); + + function testValidCancelRestart(this) + this.updateOncOutPath('output/testValidCancelRestart'); + requestId = this.onc.requestDataProduct(this.Params).dpRequestId; + runId = this.onc.runDataProduct(requestId, false).runIds(1); + responseCancel = this.onc.cancelDataProduct(requestId); + + verify_field_value(this, responseCancel, 'dpRunId', runId); + verify_field_value(this, responseCancel, 'status', 'cancelled'); + + %update MATLAB:nonExistentField error to actual http400 error for this test + %after api service fixes the issue that this 400 error does not contain "errors" field + assertError(this, @() this.onc.downloadDataProduct(runId), 'MATLAB:nonExistentField') + + runIdAfterRestart = this.onc.restartDataProduct(requestId).runIds(1); + assertEqual(this, runIdAfterRestart, runId); + + responseDownload = this.onc.downloadDataProduct(runId); + assertEqual(this, length(responseDownload), 3, "The first two are png files, and the third one is the metadata"); + verify_files_in_path(this, this.onc.outPath, 3); + verify_field_value_type(this, responseDownload(1), this.expectedFields); end end diff --git a/onc/tests/suites/Test08_RealTime.m b/onc/tests/suites/Test08_RealTime.m index 1ac730c..ce86798 100644 --- a/onc/tests/suites/Test08_RealTime.m +++ b/onc/tests/suites/Test08_RealTime.m @@ -163,7 +163,7 @@ function testDeviceValidParamsOnePage(this) result = this.onc.getDirectByDevice(this.paramsDevice); resultAllPages = this.onc.getDirectByDevice(this.paramsDeviceMultiPages, 'allPages', true); assertTrue(this, length(result.sensorData(1).data.values) > this.paramsDeviceMultiPages.rowLimit, ... - 'Test should return at least `rowLimit` rows for each sensor.'); + 'Test should return at least `rowLimit` rows.'); assertEmpty(this, result.next, 'Test should return only one page.'); assertEqual(this, resultAllPages.sensorData(1).data, result.sensorData(1).data, ... 'Test should concatenate rows for all pages.'); @@ -178,8 +178,7 @@ function testDeviceValidParamsMultiplePages(this) end %% Testing rawdata device - %{ - % waiting for python updates + function testRawDeviceInvalidParamValue(this) filters = this.paramsDevice; filters.deviceCode = 'XYZ123'; @@ -196,27 +195,29 @@ function testRawDeviceNoData(this) filters = this.paramsDevice; filters.dateFrom = '2000-01-01'; filters.dateTo = '2000-01-02'; - result = this.onc.getDirectByDevice(filters); - assertEmpty(this, result.sensorData); + result = this.onc.getDirectRawByDevice(filters); + assertEmpty(this, result.data.lineTypes); + assertEmpty(this, result.data.readings); + assertEmpty(this, result.data.times); end function testRawDeviceValidParamsOnePage(this) - result = this.onc.getDirectByDevice(this.paramsDevice); - resultAllPages = this.onc.getDirectByDevice(this.paramsDeviceMultiPages, 'allPages', true); - assertTrue(this, length(result.sensorData(1).data.values) > this.paramsDeviceMultiPages.rowLimit, ... + result = this.onc.getDirectRawByDevice(this.paramsDevice); + resultAllPages = this.onc.getDirectRawByDevice(this.paramsDeviceMultiPages, 'allPages', true); + assertTrue(this, length(result.data.readings) > this.paramsDeviceMultiPages.rowLimit, ... 'Test should return at least `rowLimit` rows for each sensor.'); assertEmpty(this, result.next, 'Test should return only one page.'); - assertEqual(this, resultAllPages.sensorData(1).data, result.sensorData(1).data, ... + assertEqual(this, resultAllPages.data, result.data, ... 'Test should concatenate rows for all pages.'); assertEmpty(this, resultAllPages.next, 'Test should return only one page.'); end function testRawDeviceValidParamsMultiplePages(this) - result = this.onc.getDirectByDevice(this.paramsDeviceMultiPages); - assertEqual(this, length(result.sensorData(1).data.values), this.paramsDeviceMultiPages.rowLimit, ... + result = this.onc.getDirectRawByDevice(this.paramsDeviceMultiPages); + assertEqual(this, length(result.data.readings), this.paramsDeviceMultiPages.rowLimit, ... 'Test should only return `rowLimit` rows for each sensor.'); assertTrue(this, ~isempty(result.next), 'Test should return multiple pages.'); end - %} + end end \ No newline at end of file From 8f94aabc85b7dc085fe48df3b989b59f0e77bc62 Mon Sep 17 00:00:00 2001 From: Isla Li Date: Thu, 18 Apr 2024 17:02:33 -0700 Subject: [PATCH 2/5] issue-11: update oncEnv from logical values to char --- .github/workflows/matlab.yml | 2 +- onc/tests/globals.m | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/matlab.yml b/.github/workflows/matlab.yml index 35a5e17..69ebcfc 100644 --- a/.github/workflows/matlab.yml +++ b/.github/workflows/matlab.yml @@ -37,5 +37,5 @@ jobs: source-folder: 'onc' env: TOKEN: ${{ secrets.TOKEN }} - ONC_ENV: true + ONC_ENV: 'prod' diff --git a/onc/tests/globals.m b/onc/tests/globals.m index daa37ac..be0c5a7 100644 --- a/onc/tests/globals.m +++ b/onc/tests/globals.m @@ -22,7 +22,9 @@ % get environment from ONC_ENV or use QA as default config.production = getenv('ONC_ENV'); - if isempty(config.production) + if strcmp(config.production, 'prod') + config.production = true; + else config.production = false; end % Set and save config From 2a8c3dc982cfa7f93906743a6bfc1d478d8ae4d7 Mon Sep 17 00:00:00 2001 From: Isla Li Date: Mon, 22 Apr 2024 11:33:42 -0700 Subject: [PATCH 3/5] issue-11: add back util function is_failed_response --- onc/+util/is_failed_response.m | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 onc/+util/is_failed_response.m diff --git a/onc/+util/is_failed_response.m b/onc/+util/is_failed_response.m new file mode 100644 index 0000000..9d417ac --- /dev/null +++ b/onc/+util/is_failed_response.m @@ -0,0 +1,23 @@ +function isFailed = is_failed_response(response, status) + %% Checks if a server response describes a failure + % + % * response: (struct) Response as returned by do_request() + % - status: (double) http status code + % + % Returns: (Logical) true when the response is a failure + isFailed = false; + + % Fail if HTTP status code is not a 2xx + if exist('status', 'var') + if status < 200 || status > 226 + isFailed = true; + return + end + end + + % Fail if the response is an error description + if isfield(response, "errors") + names = fieldnames(response.errors(1)); + isFailed = ismember("errorCode", names) && ismember("errorMessage", names); + end +end \ No newline at end of file From daf2ece0c2411ea365ded944b8b2df5dda7f8b72 Mon Sep 17 00:00:00 2001 From: Isla Li Date: Tue, 23 Apr 2024 15:20:54 -0700 Subject: [PATCH 4/5] issue-11: add success and failure messages --- onc/+onc/OncDelivery.m | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/onc/+onc/OncDelivery.m b/onc/+onc/OncDelivery.m index a24cf2a..ff97483 100644 --- a/onc/+onc/OncDelivery.m +++ b/onc/+onc/OncDelivery.m @@ -170,7 +170,9 @@ filters = struct('method', 'cancel', 'token', this.token, 'dpRequestId', dpRequestId); [response, info] = this.doRequest(url, filters); if isfield(response, 'status') && strcmp(response.status, 'cancelled') && info.status == 200 - fprintf("Data product with request id %d and run id %d was successfully cancelled\n", dpRequestId, response.dpRunId); + fprintf("The data product with request id %d and run id %d has been successfully cancelled\n", dpRequestId, response.dpRunId); + else + fprintf("Failed to cancel the data Product."); end end @@ -220,7 +222,9 @@ filters = struct('method', 'restart', 'token', this.token, 'dpRequestId', dpRequestId); [response, info] = this.doRequest(url, filters); if isfield(response, 'status') && (strcmp(response.status, 'data product running') || strcmp(response.status, 'queued')) && info.status == 200 - fprintf("Restarted data product with request id %d and run id %d\n", dpRequestId, response.dpRunId); + fprintf("The data product with request id %d and run id %d has been successfully restarted\n", dpRequestId, response.dpRunId); + else + fprintf("Failed to restart the data product"); end if waitComplete [response, info] = this.runDataProduct(dpRequestId, true); From 94c9dafbba15aa498f4b9ad3ce02170694587d44 Mon Sep 17 00:00:00 2001 From: IslaL <142735981+IslaL@users.noreply.github.com> Date: Tue, 23 Apr 2024 15:23:03 -0700 Subject: [PATCH 5/5] add new line characters --- onc/+onc/OncDelivery.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/onc/+onc/OncDelivery.m b/onc/+onc/OncDelivery.m index ff97483..bc35aae 100644 --- a/onc/+onc/OncDelivery.m +++ b/onc/+onc/OncDelivery.m @@ -170,9 +170,9 @@ filters = struct('method', 'cancel', 'token', this.token, 'dpRequestId', dpRequestId); [response, info] = this.doRequest(url, filters); if isfield(response, 'status') && strcmp(response.status, 'cancelled') && info.status == 200 - fprintf("The data product with request id %d and run id %d has been successfully cancelled\n", dpRequestId, response.dpRunId); + fprintf("The data product with request id %d and run id %d has been successfully cancelled.\n", dpRequestId, response.dpRunId); else - fprintf("Failed to cancel the data Product."); + fprintf("Failed to cancel the data Product.\n"); end end @@ -222,9 +222,9 @@ filters = struct('method', 'restart', 'token', this.token, 'dpRequestId', dpRequestId); [response, info] = this.doRequest(url, filters); if isfield(response, 'status') && (strcmp(response.status, 'data product running') || strcmp(response.status, 'queued')) && info.status == 200 - fprintf("The data product with request id %d and run id %d has been successfully restarted\n", dpRequestId, response.dpRunId); + fprintf("The data product with request id %d and run id %d has been successfully restarted.\n", dpRequestId, response.dpRunId); else - fprintf("Failed to restart the data product"); + fprintf("Failed to restart the data product.\n"); end if waitComplete [response, info] = this.runDataProduct(dpRequestId, true);