[TypeScript][Angular] fix issues with empty body

[macjohnny]

PR checklist

• Read the contribution guidelines.
• Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
• Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
• Copied the technical committee to review the pull request if your PR is targeting a particular programming langauge.

Description of the PR

fixes #6649, #6723

[macjohnny]

cc @roni-frantchi @wing328 @sebastianhaas @bedag-moo @krizic @taxpon @kenisteward @watiko @pgrm @topce

[pgrm]

the comment is wrong 😉 this is what I came up with, but wasn't fast enough to create a PR

/**
 * Check if body param is allowed for the request method
 *
 * @return true if request method is PUT, PATCH or POST; false otherwise
 */

[macjohnny]

ah, now I see it. I updated the PR accordingly

[pgrm]

@macjohnny update the comment in the java file, otherwise it looks cool, thx for creating the PR 👍

[pgrm]

@wing328 you can mark this one also with bug and typescript

[macjohnny]

@wing328 @sebastianhaas the checks passed, how about merging?

[defmonk0]

Hello! I was having some trouble with delete calls (it was passing 3 parameters, when the delete method can only take 2), so @macjohnny asked me to try this branch to see if it fixed the problem.

While it does fix the parameter mismatch, it doesn't quite fix the problem altogether. This is the API I've been using, with the contacts->delete method. As you can see, it wants a list of contact_ids in the body of the request. This PR still has an issue where it doesn't pass body data for delete requests at all (which is valid swagger, even if it's uncommon).

As I understand it, Angular does not currently support body data in the options of the delete method. As an alternative, the request method does. If we used request instead, we could have full control over what gets passed around.

I don't have much experience with mustache templates, but I also think it's worth considering using the request method for all of the call types. It would be more consistent, so it might simplify the template in the long run. I'm open to input on this, if anyone has any particular thoughts on using one over the other.

[sebastianhaas]

I don't have much experience with mustache templates, but I also think it's worth considering using the request method for all of the call types. It would be more consistent, so it might simplify the template in the long run. I'm open to input on this, if anyone has any particular thoughts on using one over the other.

@defmonk0 either that, or you could try and file a request with the Angular team. Not sure if body data in a delete is inside specs though, so they might as well not agree on doing this. Would be interesting to do some research on that, maybe I'll find some time today.

[macjohnny]

@defmonk0 @sebastianhaas the HTTP standard does not explicitly forbid body data with DELETE requests. however, it seems that the body data does not have any semantic meaning, which is why it is discouraged. See discussions e.g. here:
https://stackoverflow.com/questions/14323716/restful-alternatives-to-delete-request-body
https://stackoverflow.com/questions/299628/is-an-entity-body-allowed-for-an-http-delete-request

[sebastianhaas]

I think this might be a useful addition, but maybe @wing328 needs to step in here as this is a more general question to me, whether swagger codegen wants to enforce APIs to comply with standards or tries to maximize compatibility.

[sebastianhaas]

Do you know why you got these changes?

[macjohnny]

I guess because the v4.3 was not re-generated after the changes introduced in #6233
this means that 4.3 somehow is not checked by CI. @wing328 can you fix that?

[sebastianhaas]

And these empty lines?

[macjohnny]

same as above

[sebastianhaas]

I see, and did someone delete the newline at the end? And does your output contain one or two blank lines in the end? It appears to be two, but maybe that's just Github

[macjohnny]

@sebastianhaas i cannot tell where the newlines come from, but they also appear in e.g. #6769, so it is just about re-generating the files

[macjohnny]

@sebastianhaas so does this PR look good to you?

[sebastianhaas]

Yes, I guess so, I'm always in favor of enforcing standards, however, for a project like a codegen it might also be sensible to maximize compatibility. Maybe using an option flag though.

[macjohnny]

@sebastianhaas alright, but I suggest to merge this PR and discuss the DELETE body issue in #6723 , and eventually create another PR.

[pgrm]

@sebastianhaas / @macjohnny / @defmonk0 FYI - it seems more of an incomplete typescript restriction, than actually intended use case that DELETE would allow to set a body when using the request method, meaning that the angular team might remove this possibility in the future.

My reasoning is based on the following method signature:

request<R>(req: HttpRequest<any>): Observable<HttpEvent<R>>;

and here are the constructors for HttpRequest:

constructor(method: 'DELETE' | 'GET' | 'HEAD' | 'JSONP' | 'OPTIONS', url: string, init?: {
    headers?: HttpHeaders;
    reportProgress?: boolean;
    params?: HttpParams;
    responseType?: 'arraybuffer' | 'blob' | 'json' | 'text';
    withCredentials?: boolean;
});
constructor(method: 'POST' | 'PUT' | 'PATCH', url: string, body: T | null, init?: {
    headers?: HttpHeaders;
    reportProgress?: boolean;
    params?: HttpParams;
    responseType?: 'arraybuffer' | 'blob' | 'json' | 'text';
    withCredentials?: boolean;
});
constructor(method: string, url: string, body: T | null, init?: {
    headers?: HttpHeaders;
    reportProgress?: boolean;
    params?: HttpParams;
    responseType?: 'arraybuffer' | 'blob' | 'json' | 'text';
    withCredentials?: boolean;
});

[sebastianhaas]

@pgrm good catch, even more so, let's merge this PR and discuss options re compatibility with non-standard API definitions in another place.

[wing328]

If no more feedback/question on this, I'll merge this tomorrow (Tue)