flowup / api-client-generator

Angular REST API client generator from Swagger YAML or JSON file with camel case settigs
MIT License
115 stars 21 forks source link

[api-client] array params methods should use append instead of set #31

Closed warmans closed 6 years ago

warmans commented 6 years ago

here is an example swagger parameter definition:

{
  "type": "array",
  "items": {
    "type": "string"
  },
  "name": "filters",
  "in": "query"
}

and here is what gets generated for that parameter

  listThings(filters: string[], options?: HttpOptions): Observable<models.V2RuleList> {
    const path = `/api/v2/things`;
    options = {...this.options, ...options};

    if (filters) {
      options.params = options.params.set('filters', String(filters));
    }
    return this.sendRequest<models.V2ThingList>('GET', path, options);
  }

The filters array ends up being joined with commas.

What should happen:

The generated code for the param should look more like this:

    if (filters) {
      filters.forEach(v => {
        options.params = options.params.append('filters', v);
      });
    }

This is unfortunately arguable since apparently there is not a clear standard. However multiple params with the same name is the most and reliable approach AFAIK so it should probably be used.

The functionality could be hidden behind a flag for backwards compatibility i.e. it uses set by default but append if --append-array-params is supplied during generation.

vmasek commented 6 years ago

I think the best will be to have the option that will switch the set for append as you proposed

warmans commented 6 years ago

OK I'll probably work on a PR for this later today.

vmasek commented 6 years ago

In beta branch I have refactored the code and split it to separate functions, I am also planning to add linting. You can wait till it is in master so I wouldn't mess up your work with conflicts.