search

cyclosproject / ng-openapi-gen

An OpenAPI 3.0 codegen for Angular
MIT License
397 stars 132 forks source link

Response is always void in the generated methods. [openapi 3.0.1, ng-openapi-gen@0.20.0 #204

Closed marcellkiss closed 2 years ago

marcellkiss commented 2 years ago

I'm getting Observable<void> responses in the generated apiService methods, although I have an array of objects as a response in my openapi.json. I created a repository, to showcase the problem: https://github.com/marcellkiss/ng-openapi-gen-debug

Here's my openapi.json: https://github.com/marcellkiss/ng-openapi-gen-debug/blob/main/openapi.json#L26 And here's what gets generated: https://github.com/marcellkiss/ng-openapi-gen-debug/blob/main/output/services/api.service.ts#L68

Where's the problem?

luisfpg commented 2 years ago

According to https://swagger.io/docs/specification/describing-responses/, the "default" response is used to describe any exception response, not the success response. You need at least one 2xx response in your path for the generator to guess the success response type.

marcellkiss commented 2 years ago

After having a look at the topic again, I believe, it would make sense, to assume, that the default response should be the response, if there's nothing else defined. After reading the docs I still don't feel like, that default should be exclusive for error codes.

On the backend, we're using io.swagger.v3.oas.annotations.Operation and the response types are read automatically from the methods and put under default in the openapi.json file.

luisfpg commented 2 years ago

From that spec page: "Sometimes, an operation can return multiple errors with different HTTP status codes, but all of them have the same response structure... You can use the default response to describe these errors collectively, not individually". Seems pretty exclusive for me... ;)

marcellkiss commented 2 years ago

Hi Luis, yeah, that's clear, that's a valid use-case. But why do you think it's exclusive? The docs say, 'you can', and not 'you can only'. That's why I think there's more room for different use-cases.

Let's have a look at the interpretation of the developers of the io.swagger.v3.oas.annotations.Operation library. If you create a method, without defining the response code, it puts the response type of the method under the default key. This totally makes sense, it's default, cause nothing else was defined. On the other hand, it's also clear, that this response type is not an exception.

I can't find any line in the docs, which states, that a default response must be an exception. And if it's so, it can be a 'normal' response, if nothing else is defined.

I don't want to cause extra work, I'm also happy to make a pull-request, but first, we need consensus :)

Motivation & benefits: it spares quite a lot of boilerplate code on the backend, as long as you don't want to define detailed response codes and exceptions.

luisfpg commented 2 years ago

In many places the OpenAPI specification is loose, and causes problems. See #206. The point is that if you have just a default response, you're not documenting ANY error codes at all. It's like saying an HTTP service will never ever fail (hahahahaha). I'm not against assuming the default response to be also used as success response. My problem is finding time to support this library. I'm already struggling to keep the issues properly replied ;) If you can contribute a PR, the place to change is operation.ts, in collectResponses() method. Something like: if all responses were iterated and no success response exist, and there's a default response, assume it as success. Ah, and if so, don't forget to add some test case for this.