Bug #17319
closedCommonService.get(uuid) should throw an exception when uuid is empty string
Description
Currently if uuid
is "", the service layer does a request to the backend with an incorrect URL, for example: arvados/v1/container_requests/
, ultimately getting a 404 response.
RailsAPI may accept trailing slashes on some endpoints, but as we're migrating those endpoint handlings to controller
, these kind of issues may start to creep up of different parts of the app.
Updated by Lucas Di Pentima almost 4 years ago
- Target version changed from 2021-02-17 sprint to 2021-03-03 sprint
Updated by Lucas Di Pentima almost 4 years ago
- Status changed from New to In Progress
Updated by Lucas Di Pentima almost 4 years ago
Fix at arvados-workbench2|0429d378 - branch 17319-service-layer-uuid-validation
Test run: developer-tests-workbench2: #293
- Validates that
uuid
is not an empty string onCommonService
'sget()
,update()
anddelete()
calls. - Adds unit tests.
Updated by Daniel Kutyła almost 4 years ago
src\services\common-service\common-service.test.ts
const axiosInstance = axios.create();
// const axiosMock = new MockAdapter(axiosInstance);
const commonService = new CommonService(axiosInstance, "resource", actions);
please use mock and wrap this code in the beforeEach
Updated by Lucas Di Pentima almost 4 years ago
Updates at arvados-workbench2|6a66e260
- Moves mock's init to
beforeEach()
Updated by Daniel Kutyła almost 4 years ago
commonService = new CommonService<any>(axiosInstance, "resource", actions);
axiosInstance = axios.create();
should be
axiosInstance = axios.create();
commonService = new CommonService<any>(axiosInstance, "resource", actions);
as on a first call axiosInstance is null, on the other hand are you sure that you want to use axios without mocking within the test file?
Updated by Lucas Di Pentima almost 4 years ago
Updates at arvados-workbench2|0c04dddf
I've cleaned up the test code a little bit, the axios instance is created just to satisfy the CommonService
constructor, it's not being used on the test cases.
AFAICT, the axios mock would allow me to re-record test responses but in these test cases I'm not testing that, I'm just confirming that exceptions are being thrown when uuid==""
on some service calls.
Could you explain a little bit about your concerns? I'm not too experienced on TypeScript/Javascript testing. Thanks!
Updated by Lucas Di Pentima almost 4 years ago
Update at arvados-workbench2|eb40293b
Don't use a real AxiosInstance
when not needed, to avoid potential issue with future test authors.
Updated by Anonymous almost 4 years ago
- % Done changed from 0 to 100
- Status changed from In Progress to Resolved
Applied in changeset arvados:arvados-workbench2|1492885f32eefc4599f4f44228a6adf2ed7b8f7f.