Remove "curried" nested resources and manually specified urlParams (#625

)

* Drop support for optional url params

* Delete nested resource files

* Remove urlData

* Extract urlParams from path instead of manual definition

Verified this is no different with:

```js
const urlParams = utils.extractUrlParams(spec.path || '');
if (
  !(spec.urlParams || []).every((x, i) => urlParams[i] === x) ||
  (spec.urlParams || []).length !== urlParams.length
) {
  throw Error(
    'mismatch' + JSON.stringify(urlParams) + JSON.stringify(spec.urlParams)
  );
}
```

inside StripeMethod

* Remove manually specified urlParams

* Add a deprecation error message

* Revert "Delete nested resource files"

This reverts commit d88a3e7.

* Fix nested resources for non-curried urlParams and update tests to demonstrate their use

* Refactor makeRequest

* Revert "Revert "Delete nested resource files""

This reverts commit e5eccb8.

lib/StripeMethod.basic.js

@@ -15,19 +15,16 @@ module.exports = {
   retrieve: stripeMethod({
     method: 'GET',
     path: '/{id}',
-    urlParams: ['id'],
   }),
 
   update: stripeMethod({
     method: 'POST',
     path: '{id}',
-    urlParams: ['id'],
   }),
 
   // Avoid 'delete' keyword in JS
   del: stripeMethod({
     method: 'DELETE',
     path: '{id}',
-    urlParams: ['id'],
   }),
 };

lib/StripeMethod.js

@@ -26,6 +26,10 @@ function stripeMethod(spec) {
 
     const callback = typeof args[args.length - 1] == 'function' && args.pop();
 
+    spec.urlParams = utils.extractUrlParams(
+      self.createResourcePathWithSymbols(spec.path || '')
+    );
+
     const requestPromise = utils.callbackifyPromiseWithTimeout(
       makeRequest(self, args, spec, {}),
       callback

lib/StripeResource.js

@@ -8,8 +8,6 @@ const uuid = require('uuid/v4');
 const utils = require('./utils');
 const Error = require('./Error');
 
-const hasOwn = {}.hasOwnProperty;
-
 const defaultHttpAgent = new http.Agent({keepAlive: true});
 const defaultHttpsAgent = new https.Agent({keepAlive: true});
 
@@ -25,9 +23,13 @@ StripeResource.MAX_BUFFERED_REQUEST_METRICS = 100;
 /**
  * Encapsulates request logic for a Stripe Resource
  */
-function StripeResource(stripe, urlData) {
+function StripeResource(stripe, deprecatedUrlData) {
   this._stripe = stripe;
-  this._urlData = urlData || {};
+  if (deprecatedUrlData) {
+    throw new Error(
+      'Support for curried url params was dropped in stripe-node v7.0.0. Instead, pass two ids.'
+    );
+  }
 
   this.basePath = utils.makeURLInterpolator(
     this.basePath || stripe.getApiField('basePath')
@@ -81,17 +83,6 @@ StripeResource.prototype = {
       .replace(/\\/g, '/')}`; // ugly workaround for Windows
   },
 
-  createUrlData() {
-    const urlData = {};
-    // Merge in baseData
-    for (const i in this._urlData) {
-      if (hasOwn.call(this._urlData, i)) {
-        urlData[i] = this._urlData[i];
-      }
-    }
-    return urlData;
-  },
-
   // DEPRECATED: Here for backcompat in case users relied on this.
   wrapTimeout: utils.callbackifyPromiseWithTimeout,
 

lib/makeRequest.js

@@ -1,59 +1,31 @@
 'use strict';
 
 const utils = require('./utils');
-const OPTIONAL_REGEX = /^optional!/;
 
 function getRequestOpts(self, requestArgs, spec, overrideData) {
   // Extract spec values with defaults.
-  const commandPath =
-    typeof spec.path == 'function'
-      ? spec.path
-      : utils.makeURLInterpolator(spec.path || '');
+  const commandPath = utils.makeURLInterpolator(spec.path || '');
   const requestMethod = (spec.method || 'GET').toUpperCase();
   const urlParams = spec.urlParams || [];
   const encode = spec.encode || ((data) => data);
   const host = spec.host;
+  const path = self.createResourcePathWithSymbols(spec.path);
 
   // Don't mutate args externally.
   const args = [].slice.call(requestArgs);
 
   // Generate and validate url params.
-  const urlData = self.createUrlData();
-  for (let i = 0, l = urlParams.length; i < l; ++i) {
-    var path;
-
-    // Note that we shift the args array after every iteration so this just
-    // grabs the "next" argument for use as a URL parameter.
-    const arg = args[0];
-
-    let param = urlParams[i];
-
-    const isOptional = OPTIONAL_REGEX.test(param);
-    param = param.replace(OPTIONAL_REGEX, '');
-
-    if (param == 'id' && typeof arg !== 'string') {
-      path = self.createResourcePathWithSymbols(spec.path);
+  const urlData = urlParams.reduce((urlData, param) => {
+    const arg = args.shift();
+    if (typeof arg !== 'string') {
       throw new Error(
-        `Stripe: "id" must be a string, but got: ${typeof arg} (on API request to \`${requestMethod} ${path}\`)`
+        `Stripe: Argument "${param}" must be a string, but got: ${arg} (on API request to \`${requestMethod} ${path}\`)`
       );
     }
 
-    if (!arg) {
-      if (isOptional) {
-        urlData[param] = '';
-        continue;
-      }
-
-      path = self.createResourcePathWithSymbols(spec.path);
-      throw new Error(
-        `Stripe: Argument "${
-          urlParams[i]
-        }" required, but got: ${arg} (on API request to \`${requestMethod} ${path}\`)`
-      );
-    }
-
-    urlData[param] = args.shift();
-  }
+    urlData[param] = arg;
+    return urlData;
+  }, {});
 
   // Pull request data and options (headers, auth) from args.
   const dataFromArgs = utils.getDataFromArgs(args);
@@ -62,7 +34,6 @@ function getRequestOpts(self, requestArgs, spec, overrideData) {
 
   // Validate that there are no more args.
   if (args.length) {
-    path = self.createResourcePathWithSymbols(spec.path);
     throw new Error(
       `Stripe: Unknown arguments (${args}). Did you mean to pass an options object? See https://github.com/stripe/stripe-node/wiki/Passing-Options. (on API request to ${requestMethod} \`${path}\`)`
     );

lib/resources/Accounts.js

@@ -20,20 +20,17 @@ module.exports = StripeResource.extend({
   update: stripeMethod({
     method: 'POST',
     path: 'accounts/{id}',
-    urlParams: ['id'],
   }),
 
   // Avoid 'delete' keyword in JS
   del: stripeMethod({
     method: 'DELETE',
     path: 'accounts/{id}',
-    urlParams: ['id'],
   }),
 
   reject: stripeMethod({
     method: 'POST',
     path: 'accounts/{id}/reject',
-    urlParams: ['id'],
   }),
 
   retrieve(id) {
@@ -43,7 +40,6 @@ module.exports = StripeResource.extend({
       return stripeMethod({
         method: 'GET',
         path: 'accounts/{id}',
-        urlParams: ['id'],
       }).apply(this, arguments);
     } else {
       if (id === null || id === undefined) {
@@ -64,32 +60,27 @@ module.exports = StripeResource.extend({
   createExternalAccount: stripeMethod({
     method: 'POST',
     path: 'accounts/{accountId}/external_accounts',
-    urlParams: ['accountId'],
   }),
 
   listExternalAccounts: stripeMethod({
     method: 'GET',
     path: 'accounts/{accountId}/external_accounts',
-    urlParams: ['accountId'],
     methodType: 'list',
   }),
 
   retrieveExternalAccount: stripeMethod({
     method: 'GET',
     path: 'accounts/{accountId}/external_accounts/{externalAccountId}',
-    urlParams: ['accountId', 'externalAccountId'],
   }),
 
   updateExternalAccount: stripeMethod({
     method: 'POST',
     path: 'accounts/{accountId}/external_accounts/{externalAccountId}',
-    urlParams: ['accountId', 'externalAccountId'],
   }),
 
   deleteExternalAccount: stripeMethod({
     method: 'DELETE',
     path: 'accounts/{accountId}/external_accounts/{externalAccountId}',
-    urlParams: ['accountId', 'externalAccountId'],
   }),
 
   /**
@@ -99,7 +90,6 @@ module.exports = StripeResource.extend({
   createLoginLink: stripeMethod({
     method: 'POST',
     path: 'accounts/{accountId}/login_links',
-    urlParams: ['accountId'],
   }),
 
   /**
@@ -109,31 +99,26 @@ module.exports = StripeResource.extend({
   createPerson: stripeMethod({
     method: 'POST',
     path: 'accounts/{accountId}/persons',
-    urlParams: ['accountId'],
   }),
 
   listPersons: stripeMethod({
     method: 'GET',
     path: 'accounts/{accountId}/persons',
-    urlParams: ['accountId'],
     methodType: 'list',
   }),
 
   retrievePerson: stripeMethod({
     method: 'GET',
     path: 'accounts/{accountId}/persons/{personId}',
-    urlParams: ['accountId', 'personId'],
   }),
 
   updatePerson: stripeMethod({
     method: 'POST',
     path: 'accounts/{accountId}/persons/{personId}',
-    urlParams: ['accountId', 'personId'],
   }),
 
   deletePerson: stripeMethod({
     method: 'DELETE',
     path: 'accounts/{accountId}/persons/{personId}',
-    urlParams: ['accountId', 'personId'],
   }),
 });

lib/resources/ApplicationFees.js

@@ -11,25 +11,21 @@ module.exports = StripeResource.extend({
   createRefund: stripeMethod({
     method: 'POST',
     path: '/{feeId}/refunds',
-    urlParams: ['feeId'],
   }),
 
   listRefunds: stripeMethod({
     method: 'GET',
     path: '/{feeId}/refunds',
-    urlParams: ['feeId'],
     methodType: 'list',
   }),
 
   retrieveRefund: stripeMethod({
     method: 'GET',
     path: '/{feeId}/refunds/{refundId}',
-    urlParams: ['feeId', 'refundId'],
   }),
 
   updateRefund: stripeMethod({
     method: 'POST',
     path: '/{feeId}/refunds/{refundId}',
-    urlParams: ['feeId', 'refundId'],
   }),
 });

lib/resources/BitcoinReceivers.js

@@ -11,7 +11,6 @@ module.exports = StripeResource.extend({
   listTransactions: stripeMethod({
     method: 'GET',
     path: '/{id}/transactions',
-    urlParams: ['id'],
     methodType: 'list',
   }),
 });

lib/resources/Charges.js

@@ -11,6 +11,5 @@ module.exports = StripeResource.extend({
   capture: stripeMethod({
     method: 'POST',
     path: '/{id}/capture',
-    urlParams: ['id'],
   }),
 });

lib/resources/CreditNotes.js

@@ -11,6 +11,5 @@ module.exports = StripeResource.extend({
   voidCreditNote: stripeMethod({
     method: 'POST',
     path: '/{id}/void',
-    urlParams: ['id'],
   }),
 });

lib/resources/Customers.js

@@ -15,7 +15,6 @@ module.exports = StripeResource.extend({
   deleteDiscount: stripeMethod({
     method: 'DELETE',
     path: '/{customerId}/discount',
-    urlParams: ['customerId'],
   }),
 
   /**
@@ -25,38 +24,32 @@ module.exports = StripeResource.extend({
   createSource: stripeMethod({
     method: 'POST',
     path: '/{customerId}/sources',
-    urlParams: ['customerId'],
   }),
 
   listSources: stripeMethod({
     method: 'GET',
     path: '/{customerId}/sources',
-    urlParams: ['customerId'],
     methodType: 'list',
   }),
 
   retrieveSource: stripeMethod({
     method: 'GET',
     path: '/{customerId}/sources/{sourceId}',
-    urlParams: ['customerId', 'sourceId'],
   }),
 
   updateSource: stripeMethod({
     method: 'POST',
     path: '/{customerId}/sources/{sourceId}',
-    urlParams: ['customerId', 'sourceId'],
   }),
 
   deleteSource: stripeMethod({
     method: 'DELETE',
     path: '/{customerId}/sources/{sourceId}',
-    urlParams: ['customerId', 'sourceId'],
   }),
 
   verifySource: stripeMethod({
     method: 'POST',
     path: '/{customerId}/sources/{sourceId}/verify',
-    urlParams: ['customerId', 'sourceId'],
   }),
 
   /**
@@ -66,25 +59,21 @@ module.exports = StripeResource.extend({
   createTaxId: stripeMethod({
     method: 'POST',
     path: '/{customerId}/tax_ids',
-    urlParams: ['customerId'],
   }),
 
   deleteTaxId: stripeMethod({
     method: 'DELETE',
     path: '/{customerId}/tax_ids/{taxIdId}',
-    urlParams: ['customerId', 'taxIdId'],
   }),
 
   listTaxIds: stripeMethod({
     method: 'GET',
     path: '/{customerId}/tax_ids',
-    urlParams: ['customerId'],
     methodType: 'list',
   }),
 
   retrieveTaxId: stripeMethod({
     method: 'GET',
     path: '/{customerId}/tax_ids/{taxIdId}',
-    urlParams: ['customerId', 'taxIdId'],
   }),
 });