API method signature refactor

packages/arcgis-rest-auth/src/AuthenticatedRequestOptions.ts

@@ -0,0 +1,25 @@
+import { ApplicationSession } from "./ApplicationSession";
+import { UserSession } from "./UserSession";
+
+import { IRequestOptions } from "@esri/arcgis-rest-request";
+
+/**
+ * Used internally by packages for requests that require authentication.
+ */
+export interface IAuthenticatedRequestOptions extends IRequestOptions {
+  authentication: UserSession | ApplicationSession;
+}
+
+/**
+ * Used internally by packages for requests that require user authentication.
+ */
+export interface IUserRequestOptions extends IRequestOptions {
+  authentication: UserSession;
+}
+
+/**
+ * Used internally by packages for requests that require application authentication.
+ */
+export interface IApplicationRequestOptions extends IRequestOptions {
+  authentication: ApplicationSession;
+}

packages/arcgis-rest-auth/src/fetchToken.ts

@@ -36,7 +36,9 @@ export function fetchToken(
   url: string,
   params: IFetchTokenParams
 ): Promise<IFetchTokenResponse> {
-  return request(url, params).then((response: IFetchTokenRawResponse) => {
+  return request(url, {
+    params
+  }).then((response: IFetchTokenRawResponse) => {
     const r: IFetchTokenResponse = {
       token: response.access_token,
       username: response.username,

packages/arcgis-rest-auth/src/generateToken.ts

@@ -32,5 +32,5 @@ export function generateToken(
     params.referer = "@esri.arcgis-rest-auth";
   }
 
-  return request(url, params);
+  return request(url, { params });
 }

packages/arcgis-rest-auth/src/index.ts

@@ -2,3 +2,4 @@ export * from "./ApplicationSession";
 export * from "./UserSession";
 export * from "./fetchToken";
 export * from "./generateToken";
+export * from "./AuthenticatedRequestOptions";

packages/arcgis-rest-auth/test/fetchToken.test.ts

@@ -4,16 +4,6 @@ import { fetchToken } from "../src/index";
 const TOKEN_URL = "https://www.arcgis.com/sharing/rest/oauth2/token";
 
 describe("fetchToken()", () => {
-  let paramsSpy: jasmine.Spy;
-
-  beforeEach(() => {
-    paramsSpy = spyOn(FormData.prototype, "append").and.callThrough();
-  });
-
-  afterAll(() => {
-    paramsSpy.calls.reset();
-  });
-
   afterEach(fetchMock.restore);
 
   it("should request a token with `client_credentials`, `client_id` and `client_secret`", done => {
@@ -28,15 +18,14 @@ describe("fetchToken()", () => {
       grant_type: "client_credentials"
     })
       .then(response => {
-        const [url]: [string, RequestInit] = fetchMock.lastCall(TOKEN_URL);
-        expect(url).toEqual(TOKEN_URL);
-        expect(paramsSpy).toHaveBeenCalledWith("f", "json");
-        expect(paramsSpy).toHaveBeenCalledWith("client_id", "clientId");
-        expect(paramsSpy).toHaveBeenCalledWith("client_secret", "clientSecret");
-        expect(paramsSpy).toHaveBeenCalledWith(
-          "grant_type",
-          "client_credentials"
+        const [url, options]: [string, RequestInit] = fetchMock.lastCall(
+          TOKEN_URL
         );
+        expect(url).toEqual(TOKEN_URL);
+        expect(options.body).toContain("f=json");
+        expect(options.body).toContain("client_id=clientId");
+        expect(options.body).toContain("client_secret=clientSecret");
+        expect(options.body).toContain("grant_type=client_credentials");
         expect(response.token).toEqual("token");
         expect(response.expires).toBeGreaterThan(Date.now());
         done();
@@ -61,20 +50,19 @@ describe("fetchToken()", () => {
       grant_type: "authorization_code"
     })
       .then(response => {
-        const [url]: [string, RequestInit] = fetchMock.lastCall(TOKEN_URL);
-        expect(url).toEqual(TOKEN_URL);
-        expect(paramsSpy).toHaveBeenCalledWith("f", "json");
-
-        expect(paramsSpy).toHaveBeenCalledWith("client_id", "clientId");
-        expect(paramsSpy).toHaveBeenCalledWith(
-          "redirect_uri",
-          "https://example-app.com/redirect-uri"
+        const [url, options]: [string, RequestInit] = fetchMock.lastCall(
+          TOKEN_URL
         );
-        expect(paramsSpy).toHaveBeenCalledWith(
-          "grant_type",
-          "authorization_code"
+        expect(url).toEqual(TOKEN_URL);
+        expect(options.body).toContain("f=json");
+        expect(options.body).toContain("client_id=clientId");
+        expect(options.body).toContain(
+          `redirect_uri=${encodeURIComponent(
+            "https://example-app.com/redirect-uri"
+          )}`
         );
-        expect(paramsSpy).toHaveBeenCalledWith("code", "authorizationCode");
+        expect(options.body).toContain("grant_type=authorization_code");
+        expect(options.body).toContain("code=authorizationCode");
         expect(response.token).toEqual("token");
         expect(response.refreshToken).toEqual("refreshToken");
         expect(response.username).toEqual("Casey");

packages/arcgis-rest-auth/test/generateToken.test.ts

@@ -5,16 +5,6 @@ import { TOMORROW } from "./utils";
 const TOKEN_URL = "https://www.arcgis.com/sharing/rest/generateToken";
 
 describe("generateToken()", () => {
-  let paramsSpy: jasmine.Spy;
-
-  beforeEach(() => {
-    paramsSpy = spyOn(FormData.prototype, "append").and.callThrough();
-  });
-
-  afterAll(() => {
-    paramsSpy.calls.reset();
-  });
-
   afterEach(fetchMock.restore);
 
   it("should generate a token for a username and password", done => {
@@ -28,11 +18,13 @@ describe("generateToken()", () => {
       password: "Jones"
     })
       .then(response => {
-        const [url]: [string, RequestInit] = fetchMock.lastCall(TOKEN_URL);
+        const [url, options]: [string, RequestInit] = fetchMock.lastCall(
+          TOKEN_URL
+        );
         expect(url).toEqual(TOKEN_URL);
-        expect(paramsSpy).toHaveBeenCalledWith("f", "json");
-        expect(paramsSpy).toHaveBeenCalledWith("username", "Casey");
-        expect(paramsSpy).toHaveBeenCalledWith("password", "Jones");
+        expect(options.body).toContain("f=json");
+        expect(options.body).toContain("username=Casey");
+        expect(options.body).toContain("password=Jones");
         expect(response.token).toEqual("token");
         expect(response.expires).toEqual(TOMORROW.getTime());
         done();

packages/arcgis-rest-geocoder/src/geocoder.ts

@@ -78,7 +78,7 @@ export interface IGeocodeParams extends IParams {
   magicKey?: string;
 }
 
-export interface IGeocodeRequestOptions extends IRequestOptions {
+export interface IGenericGeocodeRequestOptions extends IRequestOptions {
   /**
    * Any ArcGIS Geocoding service (example: http://sampleserver6.arcgisonline.com/arcgis/rest/services/Locators/SanDiego/GeocodeServer )
    */
@@ -139,43 +139,38 @@ export interface IGeocodeServiceInfoResponse {
  *     response.candidates[0].location; // => { x: -118.409, y: 33.943, spatialReference: { wkid: 4326 }  }
  *   });
  *
- * geocode({address: "1600 Pennsylvania Ave", postal: 20500}, { countryCode: "USA" })
+ * geocode({address: "1600 Pennsylvania Ave", postal: 20500}, { params: { countryCode: "USA" }})
  *   .then((response) => {
  *     response.candidates[0].location; // => { x: -77.036533, y: 38.898719, spatialReference: { wkid: 4326 } }
  *   });
  * ```
  *
  * @param address | String or IAddress representing the address or Point of Interest to pass to the endpoint.
- * @param requestParams - Other arguments to pass to the endpoint.
  * @param requestOptions - Additional options for the request including authentication.
  * @returns A Promise that will resolve with the address candidates for the request.
  */
 export function geocode(
   address: IAddress | string,
-  requestParams?: IGeocodeParams,
-  requestOptions?: IGeocodeRequestOptions
+  requestOptions?: IGenericGeocodeRequestOptions
 ): Promise<IGeocodeResponse> {
-  const { endpoint }: IGeocodeRequestOptions = {
+  const options: IGenericGeocodeRequestOptions = {
     endpoint: worldGeocoder,
+    params: {},
     ...requestOptions
   };
 
-  let params: IParams = {
-    ...requestParams
-  };
-
-  // replace with ternary operator?
+  // would it be better to replace this with a ternary operator?
   if (typeof address === "string") {
-    params.singleLine = address;
+    options.params.singleLine = address;
   } else {
-    params = { ...address, ...params };
+    // why aren't the params from the request mixed in way up top??
+    options.params = { ...requestOptions.params, ...address };
   }
 
   // add spatialReference property to individual matches
   return request(
-    endpoint + "findAddressCandidates",
-    params,
-    requestOptions
+    options.endpoint + "findAddressCandidates",
+    options
   ).then(response => {
     const sr = response.spatialReference;
     response.candidates.forEach(function(candidate: {
@@ -195,33 +190,26 @@ export function geocode(
  * ```js
  * import { suggest } from '@esri/arcgis-geocoder';
  *
- * suggest("Starb")
+ * suggest({ partialText: "Starb" })
  *   .then((response) => {
  *     response.address.PlaceName; // => "Starbucks"
  *   });
  * ```
  *
- * @param partialText - The string to pass to the endpoint.
- * @param requestParams - Additional parameters to pass to the endpoint.
- * @param requestOptions - Additional options for the request including authentication.
+ * @param requestOptions - Options for the request including authentication and other optional parameters.
  * @returns A Promise that will resolve with the data from the response.
  */
 export function suggest(
   partialText: string,
-  requestParams?: IParams,
-  requestOptions?: IGeocodeRequestOptions
+  requestOptions?: IGenericGeocodeRequestOptions
 ): Promise<ISuggestResponse> {
-  const { endpoint }: IGeocodeRequestOptions = {
+  const options: IGenericGeocodeRequestOptions = {
     endpoint: worldGeocoder,
+    params: { text: partialText },
     ...requestOptions
   };
 
-  const params: IParams = {
-    text: partialText,
-    ...requestParams
-  };
-
-  return request(endpoint + "suggest", params, requestOptions);
+  return request(options.endpoint + "suggest", options);
 }
 
 /**
@@ -244,40 +232,34 @@ export function suggest(
  * ```
  *
  * @param coordinates - the location you'd like to associate an address with.
- * @param requestParams - Additional parameters to pass to the endpoint.
  * @param requestOptions - Additional options for the request including authentication.
  * @returns A Promise that will resolve with the data from the response.
  */
 export function reverseGeocode(
   coords: IPoint | ILocation | [number, number],
-  requestParams?: IParams,
-  requestOptions?: IGeocodeRequestOptions
+  requestOptions?: IGenericGeocodeRequestOptions
 ): Promise<IReverseGeocodeResponse> {
-  const { endpoint }: IGeocodeRequestOptions = {
+  const options: IGenericGeocodeRequestOptions = {
     endpoint: worldGeocoder,
+    params: {},
     ...requestOptions
   };
 
-  const params: IParams = {
-    location: null,
-    ...requestParams
-  };
-
   if (isLocationArray(coords)) {
-    params.location = coords.join();
+    options.params.location = coords.join();
   } else if (isLocation(coords)) {
     if (coords.lat) {
-      params.location = coords.long + "," + coords.lat;
+      options.params.location = coords.long + "," + coords.lat;
     }
     if (coords.latitude) {
-      params.location = coords.longitude + "," + coords.latitude;
+      options.params.location = coords.longitude + "," + coords.latitude;
     }
   } else {
-    // if input is a point, we can pass it straight through, with or without an sr
-    params.location = coords;
+    // if input is a point, we can pass it straight through, with or without a spatial reference
+    options.params.location = coords;
   }
 
-  return request(endpoint + "reverseGeocode", params, requestOptions);
+  return request(options.endpoint + "reverseGeocode", options);
 }
 
 /**
@@ -299,25 +281,23 @@ export function reverseGeocode(
  * ```
  *
  * @param addresses - The array of addresses you'd like to find the locations of
- * @param requestOptions - Additional options to pass to the geocoder.
- * @param requestParams - Additional parameters to pass through in the request to the geocoder.
+ * @param requestOptions - Additional options and parameters to pass to the geocoder.
  * @returns A Promise that will resolve with the data from the response.
  */
 export function bulkGeocode(
   addresses: IAddressBulk[],
-  requestOptions: IGeocodeRequestOptions, // POST by default (always)
-  requestParams?: IParams
+  requestOptions: IGenericGeocodeRequestOptions // must POST
 ) {
   // passing authentication is mandatory
-  const { endpoint }: IGeocodeRequestOptions = {
+  const { endpoint }: IGenericGeocodeRequestOptions = {
     endpoint: worldGeocoder,
     ...requestOptions
   };
 
-  const params: IParams = {
+  requestOptions.params = {
     forStorage: true,
     addresses: { records: null },
-    ...requestParams
+    ...requestOptions.params
   };
 
   const parsedAddresses: any[] = [];
@@ -326,15 +306,14 @@ export function bulkGeocode(
     parsedAddresses.push({ attributes: address });
   });
 
-  params.addresses.records = parsedAddresses;
+  requestOptions.params.addresses.records = parsedAddresses;
 
   if (!requestOptions.authentication) {
     return Promise.reject("bulk geocoding requires authentication");
   }
 
   return request(
     endpoint + "geocodeAddresses",
-    params,
     requestOptions
   ).then(response => {
     const sr = response.spatialReference;
@@ -361,10 +340,10 @@ export function bulkGeocode(
  * @returns A Promise that will resolve with the data from the response.
  */
 export function serviceInfo(
-  requestOptions?: IGeocodeRequestOptions
+  requestOptions?: IGenericGeocodeRequestOptions
 ): Promise<IGeocodeServiceInfoResponse> {
   const url = (requestOptions && requestOptions.endpoint) || worldGeocoder;
-  return request(url, {}, requestOptions);
+  return request(url, requestOptions);
 }
 
 export default {