fix(arcgis-rest-portal): wrong parameters for file upload APIs (#761)

The addItemPart and Commit API were using the wrong request parameters. This PR makes sure request parameters are correct for AGOL/Portal. AFFECTS PACKAGES: @esri/arcgis-rest-portal ISSUES CLOSED: #693, #694, #700
Esri · Oct 2, 2020 · cbfef7d · cbfef7d
1 parent d313545
commit cbfef7d
Show file tree

Hide file tree

Showing 7 changed files with 217 additions and 155 deletions.
diff --git a/packages/arcgis-rest-feature-layer/test/attachments.test.ts b/packages/arcgis-rest-feature-layer/test/attachments.test.ts
@@ -28,7 +28,7 @@ export function attachmentFile() {
   } else {
     const fs = require("fs");
     return fs.createReadStream(
-      "./packages/arcgis-rest-feature-service/test/mocks/foo.txt"
+      "./packages/arcgis-rest-feature-layer/test/mocks/foo.txt"
     );
   }
 }

diff --git a/packages/arcgis-rest-portal/src/items/add.ts b/packages/arcgis-rest-portal/src/items/add.ts
@@ -10,8 +10,7 @@ import {
   IUpdateItemResponse,
   IItemResourceResponse,
   determineOwner,
-  IManageItemRelationshipOptions,
-  IItemPartOptions
+  IManageItemRelationshipOptions
 } from "./helpers";
 import { updateItem, IUpdateItemOptions } from "./update";
 
@@ -137,39 +136,3 @@ export function addItemResource(
     return request(url, requestOptions);
   });
 }
-
-/**
- * ```js
- * import { addItemPart } from "@esri/arcgis-rest-portal";
- * //
- * addItemPart({
- *   id: "30e5fe3149c34df1ba922e6f5bbf808f",
- *   part: data,
- *   partNum: 1,
- *   authentication
- * })
- *   .then(response)
- * ```
- * Inquire about status when publishing an item, adding an item in async mode, or adding with a multipart upload. See the [REST Documentation](https://developers.arcgis.com/rest/users-groups-and-items/add-item-part.htm) for more information.
- *
- * @param id - The Id of the item to get status for.
- * @param requestOptions - Options for the request
- * @returns A Promise to get the item status.
- */
-export function addItemPart(
-  requestOptions?: IItemPartOptions
-): Promise<IUpdateItemResponse> {
-  return determineOwner(requestOptions).then(owner => {
-    const url = `${getPortalUrl(requestOptions)}/content/users/${owner}/items/${
-      requestOptions.id
-    }/addPart`;
-
-    const options = appendCustomParams<IItemPartOptions>(
-      requestOptions,
-      ["file", "partNum"],
-      { params: { ...requestOptions.params } }
-    );
-
-    return request(url, options);
-  });
-}
diff --git a/packages/arcgis-rest-portal/src/items/create.ts b/packages/arcgis-rest-portal/src/items/create.ts
@@ -79,15 +79,9 @@ export function createFolder(
 export function createItemInFolder(
   requestOptions: ICreateItemOptions
 ): Promise<ICreateItemResponse> {
-  if (requestOptions.file && !requestOptions.multipart) {
-    return Promise.reject(
-      new Error("The request must be a multipart request for file uploading.")
-    );
-  }
-
   if (requestOptions.multipart && !requestOptions.filename) {
     return Promise.reject(
-      new Error("The file name is required for a multipart request.")
+      new Error("The filename is required for a multipart request.")
     );
   }
 

diff --git a/packages/arcgis-rest-portal/src/items/upload.ts b/packages/arcgis-rest-portal/src/items/upload.ts
@@ -1,15 +1,64 @@
 /* Copyright (c) 2017-2019 Environmental Systems Research Institute, Inc.
  * Apache-2.0 */
 
-import { request } from "@esri/arcgis-rest-request";
+import { request, appendCustomParams } from "@esri/arcgis-rest-request";
+import { IItemAdd } from "@esri/arcgis-rest-types";
 
 import { getPortalUrl } from "../util/get-portal-url";
 import {
   IUserItemOptions,
   IUpdateItemResponse,
-  determineOwner
+  determineOwner,
+  IItemPartOptions,
+  serializeItem
 } from "./helpers";
 
+export interface ICommitItemOptions extends IUserItemOptions {
+  item: IItemAdd;
+}
+
+/**
+ * ```js
+ * import { addItemPart } from "@esri/arcgis-rest-portal";
+ * //
+ * addItemPart({
+ *   id: "30e5fe3149c34df1ba922e6f5bbf808f",
+ *   file: data,
+ *   partNum: 1,
+ *   authentication
+ * })
+ *   .then(response)
+ * ```
+ * Add Item Part allows the caller to upload a file part when doing an add or update item operation in multipart mode. See the [REST Documentation](https://developers.arcgis.com/rest/users-groups-and-items/add-item-part.htm) for more information.
+ *
+ * @param requestOptions - Options for the request
+ * @returns A Promise to add the item part status.
+ */
+export function addItemPart(
+  requestOptions?: IItemPartOptions
+): Promise<IUpdateItemResponse> {
+  const partNum = requestOptions.partNum;
+
+  if (!Number.isInteger(partNum) || partNum < 1 || partNum > 10000) {
+    return Promise.reject(new Error('The part number must be an integer between 1 to 10000, inclusive.'))
+  }
+
+  return determineOwner(requestOptions).then(owner => {
+    // AGO adds the "partNum" parameter in the query string, not in the body
+    const url = `${getPortalUrl(requestOptions)}/content/users/${owner}/items/${
+      requestOptions.id
+    }/addPart?partNum=${partNum}`;
+
+    const options = appendCustomParams<IItemPartOptions>(
+      requestOptions,
+      ["file"],
+      { params: { ...requestOptions.params } }
+    );
+
+    return request(url, options);
+  });
+}
+
 /**
  * ```js
  * import { commitItemUpload } from "@esri/arcgis-rest-portal";
@@ -22,19 +71,29 @@ import {
  * ```
  * Commit is called once all parts are uploaded during a multipart Add Item or Update Item operation. See the [REST Documentation](https://developers.arcgis.com/rest/users-groups-and-items/commit.htm) for more information.
  *
- * @param id - The Id of the item to commit upload for.
  * @param requestOptions - Options for the request
  * @returns A Promise to get the commit result.
  */
 export function commitItemUpload(
-  requestOptions?: IUserItemOptions
+  requestOptions?: ICommitItemOptions
 ): Promise<IUpdateItemResponse> {
   return determineOwner(requestOptions).then(owner => {
     const url = `${getPortalUrl(requestOptions)}/content/users/${owner}/items/${
       requestOptions.id
     }/commit`;
 
-    return request(url, requestOptions);
+    const options = appendCustomParams<ICommitItemOptions>(
+      requestOptions,
+      [],
+      {
+        params: {
+          ...requestOptions.params,
+          ...serializeItem(requestOptions.item)
+        }
+      }
+    );
+
+    return request(url, options);
   });
 }
 
@@ -50,7 +109,6 @@ export function commitItemUpload(
  * ```
  * Cancels a multipart upload on an item. See the [REST Documentation](https://developers.arcgis.com/rest/users-groups-and-items/cancel.htm) for more information.
  *
- * @param id - The Id of the item to cancel upload for.
  * @param requestOptions - Options for the request
  * @returns A Promise to get the commit result.
  */

diff --git a/packages/arcgis-rest-portal/test/items/add.test.ts b/packages/arcgis-rest-portal/test/items/add.test.ts
@@ -8,8 +8,7 @@ import { attachmentFile } from "../../../arcgis-rest-feature-layer/test/attachme
 import {
   addItemData,
   addItemResource,
-  addItemRelationship,
-  addItemPart
+  addItemRelationship
 } from "../../src/items/add";
 
 import { ItemSuccessResponse } from "../mocks/items/item";
@@ -320,82 +319,5 @@ describe("search", () => {
           fail(e);
         });
     });
-
-    it("should add a binary part to an item", done => {
-      fetchMock.once("*", {
-        success: true
-      });
-
-      const file = attachmentFile();
-
-      addItemPart({
-        id: "3ef",
-        // File() is only available in the browser
-        file,
-        partNum: 1,
-        ...MOCK_USER_REQOPTS
-      })
-        .then(() => {
-          expect(fetchMock.called()).toEqual(true);
-          const [url, options]: [string, RequestInit] = fetchMock.lastCall("*");
-          expect(url).toEqual(
-            "https://myorg.maps.arcgis.com/sharing/rest/content/users/casey/items/3ef/addPart"
-          );
-          expect(options.method).toBe("POST");
-          expect(options.body instanceof FormData).toBeTruthy();
-          const params = options.body as FormData;
-
-          if (params.get) {
-            expect(params.get("token")).toEqual("fake-token");
-            expect(params.get("f")).toEqual("json");
-            expect(params.get("file")).toEqual(file);
-            expect(params.get("partNum")).toEqual("1");
-          }
-
-          done();
-        })
-        .catch(e => {
-          fail(e);
-        });
-    });
-
-    it("should add a binary part to an item with the owner parameter", done => {
-      fetchMock.once("*", {
-        success: true
-      });
-
-      const file = attachmentFile();
-
-      addItemPart({
-        id: "3ef",
-        owner: "joe",
-        // File() is only available in the browser
-        file,
-        partNum: 1,
-        ...MOCK_USER_REQOPTS
-      })
-        .then(() => {
-          expect(fetchMock.called()).toEqual(true);
-          const [url, options]: [string, RequestInit] = fetchMock.lastCall("*");
-          expect(url).toEqual(
-            "https://myorg.maps.arcgis.com/sharing/rest/content/users/joe/items/3ef/addPart"
-          );
-          expect(options.method).toBe("POST");
-          expect(options.body instanceof FormData).toBeTruthy();
-          const params = options.body as FormData;
-
-          if (params.get) {
-            expect(params.get("token")).toEqual("fake-token");
-            expect(params.get("f")).toEqual("json");
-            expect(params.get("file")).toEqual(file);
-            expect(params.get("partNum")).toEqual("1");
-          }
-
-          done();
-        })
-        .catch(e => {
-          fail(e);
-        });
-    });
   }); // auth requests
 });
diff --git a/packages/arcgis-rest-portal/test/items/create.test.ts b/packages/arcgis-rest-portal/test/items/create.test.ts
@@ -372,29 +372,6 @@ describe("search", () => {
         });
     });
 
-    it("should throw an error for a file upload request with multipart=false", done => {
-      fetchMock.once("*", ItemSuccessResponse);
-      const fakeItem = {
-        owner: "casey",
-        title: "my fake item",
-        type: "Web Mapping Application"
-      };
-      createItemInFolder({
-        item: fakeItem,
-        file: "some file",
-        // multipart is required to be true for file upload
-        multipart: false,
-        ...MOCK_USER_REQOPTS
-      })
-        .then(() => {
-          fail();
-        })
-        .catch(() => {
-          expect(fetchMock.called()).toEqual(false);
-          done();
-        });
-    });
-
     it("should throw an error for a multipart request with no file name", done => {
       fetchMock.once("*", ItemSuccessResponse);
       const fakeItem = {