Skip to content

Commit

Permalink
Remove "curried" nested resources and manually specified urlParams (#625
Browse files Browse the repository at this point in the history
)

* 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.
  • Loading branch information
rattrayalex-stripe committed May 14, 2019
1 parent 4af9522 commit 3c99464
Show file tree
Hide file tree
Showing 44 changed files with 46 additions and 620 deletions.
3 changes: 0 additions & 3 deletions lib/StripeMethod.basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
}),
};
4 changes: 4 additions & 0 deletions lib/StripeMethod.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 6 additions & 15 deletions lib/StripeResource.js
Original file line number Diff line number Diff line change
Expand Up @@ -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});

Expand All @@ -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')
Expand Down Expand Up @@ -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,

Expand Down
47 changes: 9 additions & 38 deletions lib/makeRequest.js
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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}\`)`
);
Expand Down
18 changes: 0 additions & 18 deletions lib/resources/Accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -64,20 +60,17 @@ module.exports = StripeResource.extend({
listCapabilities: stripeMethod({
method: 'GET',
path: 'accounts/{accountId}/capabilities',
urlParams: ['accountId'],
methodType: 'list',
}),

retrieveCapability: stripeMethod({
method: 'GET',
path: 'accounts/{accountId}/capabilities/{capabilityId}',
urlParams: ['accountId', 'capabilityId'],
}),

updateCapability: stripeMethod({
method: 'POST',
path: 'accounts/{accountId}/capabilities/{capabilityId}',
urlParams: ['accountId', 'capabilityId'],
}),

/**
Expand All @@ -87,32 +80,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'],
}),

/**
Expand All @@ -122,7 +110,6 @@ module.exports = StripeResource.extend({
createLoginLink: stripeMethod({
method: 'POST',
path: 'accounts/{accountId}/login_links',
urlParams: ['accountId'],
}),

/**
Expand All @@ -132,31 +119,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'],
}),
});
21 changes: 0 additions & 21 deletions lib/resources/ApplicationFeeRefunds.js

This file was deleted.

4 changes: 0 additions & 4 deletions lib/resources/ApplicationFees.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
}),
});
1 change: 0 additions & 1 deletion lib/resources/BitcoinReceivers.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ module.exports = StripeResource.extend({
listTransactions: stripeMethod({
method: 'GET',
path: '/{id}/transactions',
urlParams: ['id'],
methodType: 'list',
}),
});
9 changes: 0 additions & 9 deletions lib/resources/Capabilities.js

This file was deleted.

1 change: 0 additions & 1 deletion lib/resources/Charges.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,5 @@ module.exports = StripeResource.extend({
capture: stripeMethod({
method: 'POST',
path: '/{id}/capture',
urlParams: ['id'],
}),
});
1 change: 0 additions & 1 deletion lib/resources/CreditNotes.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,5 @@ module.exports = StripeResource.extend({
voidCreditNote: stripeMethod({
method: 'POST',
path: '/{id}/void',
urlParams: ['id'],
}),
});
Loading

0 comments on commit 3c99464

Please sign in to comment.