From d1400d750add1a427bf2def7ed61abe213b147c5 Mon Sep 17 00:00:00 2001 From: Lucas Di Pentima Date: Tue, 11 May 2021 12:03:04 -0300 Subject: [PATCH] 17582: Fixes bug by avoiding an JSON param decoding issue on controller. Makes create and update calls send params in the form of: {rsc_name: { data }} ...to controller to avoid having problems with string params starting with opening brackets of braces and confusing controller. Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima --- .../common-resource-service.test.ts | 36 +++++++++---------- .../common-service/common-resource-service.ts | 13 +++++-- src/services/common-service/common-service.ts | 14 ++++---- .../project-service/project-service.test.ts | 6 ++-- 4 files changed, 40 insertions(+), 29 deletions(-) diff --git a/src/services/common-service/common-resource-service.test.ts b/src/services/common-service/common-resource-service.test.ts index d00412b8..a7fdac62 100644 --- a/src/services/common-service/common-resource-service.test.ts +++ b/src/services/common-service/common-resource-service.test.ts @@ -32,50 +32,50 @@ describe("CommonResourceService", () => { it("#create", async () => { axiosMock - .onPost("/resource") + .onPost("/resources") .reply(200, { owner_uuid: "ownerUuidValue" }); - const commonResourceService = new CommonResourceService(axiosInstance, "resource", actions); + const commonResourceService = new CommonResourceService(axiosInstance, "resources", actions); const resource = await commonResourceService.create({ ownerUuid: "ownerUuidValue" }); expect(resource).toEqual({ ownerUuid: "ownerUuidValue" }); }); it("#create maps request params to snake case", async () => { axiosInstance.post = jest.fn(() => Promise.resolve({data: {}})); - const commonResourceService = new CommonResourceService(axiosInstance, "resource", actions); + const commonResourceService = new CommonResourceService(axiosInstance, "resources", actions); await commonResourceService.create({ ownerUuid: "ownerUuidValue" }); - expect(axiosInstance.post).toHaveBeenCalledWith("/resource", {owner_uuid: "ownerUuidValue"}); + expect(axiosInstance.post).toHaveBeenCalledWith("/resources", {resource: {owner_uuid: "ownerUuidValue"}}); }); it("#create ignores fields listed as readonly", async () => { axiosInstance.post = jest.fn(() => Promise.resolve({data: {}})); - const commonResourceService = new CommonResourceService(axiosInstance, "resource", actions); + const commonResourceService = new CommonResourceService(axiosInstance, "resources", actions); // UUID fields are read-only on all resources. await commonResourceService.create({ uuid: "this should be ignored", ownerUuid: "ownerUuidValue" }); - expect(axiosInstance.post).toHaveBeenCalledWith("/resource", {owner_uuid: "ownerUuidValue"}); + expect(axiosInstance.post).toHaveBeenCalledWith("/resources", {resource: {owner_uuid: "ownerUuidValue"}}); }); it("#update ignores fields listed as readonly", async () => { axiosInstance.put = jest.fn(() => Promise.resolve({data: {}})); - const commonResourceService = new CommonResourceService(axiosInstance, "resource", actions); + const commonResourceService = new CommonResourceService(axiosInstance, "resources", actions); // UUID fields are read-only on all resources. await commonResourceService.update('resource-uuid', { uuid: "this should be ignored", ownerUuid: "ownerUuidValue" }); - expect(axiosInstance.put).toHaveBeenCalledWith("/resource/resource-uuid", {owner_uuid: "ownerUuidValue"}); + expect(axiosInstance.put).toHaveBeenCalledWith("/resources/resource-uuid", {resource: {owner_uuid: "ownerUuidValue"}}); }); it("#delete", async () => { axiosMock - .onDelete("/resource/uuid") + .onDelete("/resources/uuid") .reply(200, { deleted_at: "now" }); - const commonResourceService = new CommonResourceService(axiosInstance, "resource", actions); + const commonResourceService = new CommonResourceService(axiosInstance, "resources", actions); const resource = await commonResourceService.delete("uuid"); expect(resource).toEqual({ deletedAt: "now" }); }); it("#get", async () => { axiosMock - .onGet("/resource/uuid") + .onGet("/resources/uuid") .reply(200, { modified_at: "now", properties: { @@ -83,7 +83,7 @@ describe("CommonResourceService", () => { } }); - const commonResourceService = new CommonResourceService(axiosInstance, "resource", actions); + const commonResourceService = new CommonResourceService(axiosInstance, "resources", actions); const resource = await commonResourceService.get("uuid"); // Only first level keys are mapped to camel case expect(resource).toEqual({ @@ -96,7 +96,7 @@ describe("CommonResourceService", () => { it("#list", async () => { axiosMock - .onGet("/resource") + .onGet("/resources") .reply(200, { kind: "kind", offset: 2, @@ -110,7 +110,7 @@ describe("CommonResourceService", () => { items_available: 20 }); - const commonResourceService = new CommonResourceService(axiosInstance, "resource", actions); + const commonResourceService = new CommonResourceService(axiosInstance, "resources", actions); const resource = await commonResourceService.list({ limit: 10, offset: 1 }); // First level keys are mapped to camel case inside "items" arrays expect(resource).toEqual({ @@ -129,10 +129,10 @@ describe("CommonResourceService", () => { it("#list using POST when query string is too big", async () => { axiosMock - .onAny("/resource") + .onAny("/resources") .reply(200); const tooBig = 'x'.repeat(1500); - const commonResourceService = new CommonResourceService(axiosInstance, "resource", actions); + const commonResourceService = new CommonResourceService(axiosInstance, "resources", actions); await commonResourceService.list({ filters: tooBig }); expect(axiosMock.history.get.length).toBe(0); expect(axiosMock.history.post.length).toBe(1); @@ -142,10 +142,10 @@ describe("CommonResourceService", () => { it("#list using GET when query string is not too big", async () => { axiosMock - .onAny("/resource") + .onAny("/resources") .reply(200); const notTooBig = 'x'.repeat(1480); - const commonResourceService = new CommonResourceService(axiosInstance, "resource", actions); + const commonResourceService = new CommonResourceService(axiosInstance, "resources", actions); await commonResourceService.list({ filters: notTooBig }); expect(axiosMock.history.post.length).toBe(0); expect(axiosMock.history.get.length).toBe(1); diff --git a/src/services/common-service/common-resource-service.ts b/src/services/common-service/common-resource-service.ts index bc24f227..83af1e13 100644 --- a/src/services/common-service/common-resource-service.ts +++ b/src/services/common-service/common-resource-service.ts @@ -3,6 +3,7 @@ // SPDX-License-Identifier: AGPL-3.0 import { AxiosInstance } from "axios"; +import * as _ from "lodash"; import { Resource } from "src/models/resource"; import { ApiActions } from "~/services/api/api-actions"; import { CommonService } from "~/services/common-service/common-service"; @@ -26,17 +27,25 @@ export class CommonResourceService extends CommonService } create(data?: Partial) { + let payload: any; if (data !== undefined) { this.readOnlyFields.forEach( field => delete data[field] ); + payload = { + [this.resourceType.slice(0, -1)]: CommonService.mapKeys(_.snakeCase)(data), + }; } - return super.create(data); + return super.create(payload); } update(uuid: string, data: Partial) { + let payload: any; if (data !== undefined) { this.readOnlyFields.forEach( field => delete data[field] ); + payload = { + [this.resourceType.slice(0, -1)]: CommonService.mapKeys(_.snakeCase)(data), + }; } - return super.update(uuid, data); + return super.update(uuid, payload); } } diff --git a/src/services/common-service/common-service.ts b/src/services/common-service/common-service.ts index e43f9f8f..34bcb4c3 100644 --- a/src/services/common-service/common-service.ts +++ b/src/services/common-service/common-service.ts @@ -42,7 +42,7 @@ export class CommonService { constructor(serverApi: AxiosInstance, resourceType: string, actions: ApiActions, readOnlyFields: string[] = []) { this.serverApi = serverApi; - this.resourceType = '/' + resourceType; + this.resourceType = resourceType; this.actions = actions; this.readOnlyFields = readOnlyFields; } @@ -97,7 +97,7 @@ export class CommonService { create(data?: Partial, showErrors?: boolean) { return CommonService.defaultResponse( this.serverApi - .post(this.resourceType, data && CommonService.mapKeys(_.snakeCase)(data)), + .post(`/${this.resourceType}`, data && CommonService.mapKeys(_.snakeCase)(data)), this.actions, true, // mapKeys showErrors @@ -108,7 +108,7 @@ export class CommonService { this.validateUuid(uuid); return CommonService.defaultResponse( this.serverApi - .delete(this.resourceType + '/' + uuid), + .delete(`/${this.resourceType}/${uuid}`), this.actions ); } @@ -117,7 +117,7 @@ export class CommonService { this.validateUuid(uuid); return CommonService.defaultResponse( this.serverApi - .get(this.resourceType + '/' + uuid), + .get(`/${this.resourceType}/${uuid}`), this.actions, true, // mapKeys showErrors @@ -134,7 +134,7 @@ export class CommonService { if (QueryString.stringify(params).length <= 1500) { return CommonService.defaultResponse( - this.serverApi.get(this.resourceType, { params }), + this.serverApi.get(`/${this.resourceType}`, { params }), this.actions ); } else { @@ -147,7 +147,7 @@ export class CommonService { } }); return CommonService.defaultResponse( - this.serverApi.post(this.resourceType, formData, { + this.serverApi.post(`/${this.resourceType}`, formData, { params: { _method: 'GET' } @@ -161,7 +161,7 @@ export class CommonService { this.validateUuid(uuid); return CommonService.defaultResponse( this.serverApi - .put(this.resourceType + '/' + uuid, data && CommonService.mapKeys(_.snakeCase)(data)), + .put(`/${this.resourceType}/${uuid}`, data && CommonService.mapKeys(_.snakeCase)(data)), this.actions ); } diff --git a/src/services/project-service/project-service.test.ts b/src/services/project-service/project-service.test.ts index 3634b8cb..71e8b6d0 100644 --- a/src/services/project-service/project-service.test.ts +++ b/src/services/project-service/project-service.test.ts @@ -19,8 +19,10 @@ describe("CommonResourceService", () => { const projectService = new ProjectService(axiosInstance, actions); const resource = await projectService.create({ name: "nameValue" }); expect(axiosInstance.post).toHaveBeenCalledWith("/groups", { - name: "nameValue", - group_class: "project" + group: { + name: "nameValue", + group_class: "project" + } }); }); -- 2.30.2