Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow passing additional options to curl command for "Try it out" #6288

Merged
merged 6 commits into from Sep 22, 2020

Conversation

adamrdavid
Copy link
Contributor

@adamrdavid adamrdavid commented Aug 6, 2020

Description

Updates curlify.js to check for curlOptions on the request object and passes them in as options in between curl and -X

Allows requestInterceptor to add options to the curl command.
For example:

requestInterceptor: function (request) {
  if (request.method === 'GET') {
    request.curlOptions = ['-g']
    request.url = request.url
      .replace('%5B', '[')
      .replace('%5D', ']')
      .replace('%2C', ',');
  }
  return request;
}

Motivation and Context

Fixes #5537
The issue mentions other use cases (like adding cookies), but for my use case I just want to be able to pass the -g option so the curl command looks better for nested url parameters (like the fields parameter in jsonapi spec).

This allows you to use the existing requestInterceptor to add any additional options to an array attribute on the request object.

I don't think this actually changes the behavior of the request, but updating what is displayed in the curl command is enough for my use case. (actually I'd prefer an option to remove the execute part of the "try it out" function and just display the curl command)

How Has This Been Tested?

I have added 2 tests to curlify.js: 1 for passing a single option, 1 for passing multiple options with an argument.
The existing tests here ensure that this is backwards compatible when not passing any extra options.

For testing locally I just used the main api repo I work in and changed the src to my local copy.

Screenshots (if appropriate):

Screen Shot 2020-08-06 at 3 12 00 PM

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Seeking guidance on where would be good to add this. I was thinking in the configuration documentation under requestInterceptor, but that might get crowded.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
    I think so, but I'm not very familiar with the codebase.
  • All new and existing tests passed.

Allows `requestInterceptor` to add options to the curl command.
For example:

```js
requestInterceptor: function (request) {
  if (request.method === 'GET') {
    request.curlOptions = ['-g']
    request.url = request.url
      .replace('%5B', '[')
      .replace('%5D', ']')
      .replace('%2C', ',');
  }
  return request;
}
```
@adamrdavid adamrdavid changed the title Allow curl options Allow passing additional options to curl command for "Try it out" Aug 6, 2020
@adamrdavid adamrdavid marked this pull request as ready for review August 6, 2020 22:50
@tim-lai tim-lai self-assigned this Aug 14, 2020
@tim-lai
Copy link
Contributor

tim-lai commented Sep 21, 2020

@adamrdavid Thanks for the PR! Functionally looks good. However, can you migrate the newly added tests to test/unit/core/curlify.js? There was a recent migration from Mocha to Jest. Thanks!

@adamrdavid
Copy link
Contributor Author

adamrdavid commented Sep 21, 2020

Thanks @tim-lai I've updated the tests.

I was also wondering if you'd consider a pr that optionally disables the execute part of the Try it out section. I have not yet attempted this and don't know if it has been considered before.

@tim-lai tim-lai merged commit cbe99c8 into swagger-api:master Sep 22, 2020
@tim-lai
Copy link
Contributor

tim-lai commented Sep 22, 2020

@adamrdavid PR merged! Thanks for the contribution! Fyi, the generation of the curl sample occurs and is dependent on execute process.

@adamrdavid adamrdavid deleted the allow-curl-options branch September 22, 2020 19:25
@adamrdavid
Copy link
Contributor Author

Thank you @tim-lai.

I was thinking of (based on a setting somewhere) making the Try it out button itself call the curlify code and display it without the execute button. My use-case is mostly around not wanting to enable CORS on the main API, but still wanting the curl examples to show up. Should I open up an issue to discuss this or is it too far off the mark?

@tim-lai
Copy link
Contributor

tim-lai commented Sep 23, 2020

@adamrdavid I suggest creating a new feature request to generate curl sample only (if it doesn't already exist). I would create a new button, and would not attach the feature to Try It Out or Execute.

@MingweiSamuel
Copy link
Contributor

MingweiSamuel commented Oct 12, 2021

The documentation for this feature seems a little weird:

Parameters with dots in their names are single strings used to organize subordinate parameters, and are not indicative of a nested structure.

Parameters with dots in their names are single strings used to organize subordinate parameters, and are not indicative of a nested structure.

Parameter name Docker variable Description
request.curlOptions Unavailable Array. If set, MUST be an array of command line options available to the curl command. This can be set on the mutated request in the requestInterceptor function. For example request.curlOptions = ["-g", "--limit-rate 20k"]

<a name="request.curlOptions"></a>`request.curlOptions` | _Unavailable_ | `Array`. If set, MUST be an array of command line options available to the `curl` command. This can be set on the mutated request in the `requestInterceptor` function. For example `request.curlOptions = ["-g", "--limit-rate 20k"]`


So those seems to suggest you'd use this as follows:

Swagger({
  'request.curlOptions': [ '--insecure' ],
  ...
});

When in reality it seems you have to do this:

Swagger({
  requestInterceptor: (request) => {
    request.curlOptions = [ '--insecure' ];
    return request;
  },
  ...
});

I'm not sure if the actual request object is documented anywhere (and it's given as any { [prop: string]: any; } in the .d.ts), but I believe that is where the doc row above ought to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need more options for the generated curl command
3 participants