17582: Fixes bug by avoiding an JSON param decoding issue on controller.
authorLucas Di Pentima <lucas@di-pentima.com.ar>
Tue, 11 May 2021 15:03:04 +0000 (12:03 -0300)
committerLucas Di Pentima <lucas@di-pentima.com.ar>
Tue, 11 May 2021 16:00:40 +0000 (13:00 -0300)
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 <lucas@di-pentima.com.ar>

src/services/common-service/common-resource-service.test.ts
src/services/common-service/common-resource-service.ts
src/services/common-service/common-service.ts
src/services/project-service/project-service.test.ts

index d00412b8a69baa3941ec0bccca56f882928e0a6a..a7fdac621f18ea9a985a1191f308b7c83a9fbc50 100644 (file)
@@ -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);
index bc24f22796b21001bce09b358e1efe6c657d6248..83af1e13acdc6af6b9a4133e17fbcd1741df3e1d 100644 (file)
@@ -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<T extends Resource> extends CommonService<T>
     }
 
     create(data?: Partial<T>) {
+        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<T>) {
+        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);
     }
 }
 
index e43f9f8f136a7404af40b9ffb0626cffd650d9ad..34bcb4c3bb72708915d618ade9bff5ef7e724f63 100644 (file)
@@ -42,7 +42,7 @@ export class CommonService<T> {
 
     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<T> {
     create(data?: Partial<T>, showErrors?: boolean) {
         return CommonService.defaultResponse(
             this.serverApi
-                .post<T>(this.resourceType, data && CommonService.mapKeys(_.snakeCase)(data)),
+                .post<T>(`/${this.resourceType}`, data && CommonService.mapKeys(_.snakeCase)(data)),
             this.actions,
             true, // mapKeys
             showErrors
@@ -108,7 +108,7 @@ export class CommonService<T> {
         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<T> {
         this.validateUuid(uuid);
         return CommonService.defaultResponse(
             this.serverApi
-                .get<T>(this.resourceType + '/' + uuid),
+                .get<T>(`/${this.resourceType}/${uuid}`),
             this.actions,
             true, // mapKeys
             showErrors
@@ -134,7 +134,7 @@ export class CommonService<T> {
 
         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<T> {
                 }
             });
             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<T> {
         this.validateUuid(uuid);
         return CommonService.defaultResponse(
             this.serverApi
-                .put<T>(this.resourceType + '/' + uuid, data && CommonService.mapKeys(_.snakeCase)(data)),
+                .put<T>(`/${this.resourceType}/${uuid}`, data && CommonService.mapKeys(_.snakeCase)(data)),
             this.actions
         );
     }
index 3634b8cba60a3fc84621b4f12ef87c56ad9b53b6..71e8b6d086843bbaf983cd8f9f70791f1705ab45 100644 (file)
@@ -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"
+            }
         });
     });