Open ahmednfwela opened 1 year ago
@jaumard (2018/09) @josh-burton (2019/12) @amondnet (2019/12) @sbu-WBT (2020/12) @kuhnroyal (2020/12) @agilob (2020/12) @ahmednfwela (2021/08)
for the first issue, we introduce 2 method signatures (raw + json, as described in my comment)
I don't like the idea of mixing the methods. What if we generate a raw API instance, only depending on the HTTP framework dio|http but totally independent of any serialization library. And then we use this inside our normal API classes.
class PetApi {
final PetApiRaw rawApi;
...
Future<Response<Pet>> getPetById(String id, ...) async {
// serialize ...
final rawResponse = await rawApi.getPetById(id);
/// deserialize ...
return response;
}
}
This would still allow consumers to construct/use/extent the raw API but it doesn't clutter the typed API classes.
I am in favor of this as well, this will also minimize breaking changes
@kuhnroyal please check this PR https://github.com/OpenAPITools/openapi-generator/pull/15485
@ahmednfwela I thought about this for a while, but had not time to thoroughly go through your PRs. But due to the limited amount of time left for 7.0.0, I think we should do this in a new dart-next
experimental generator. The changes seem to big otherwise. WDYT?
@kuhnroyal I think this separation is critical right now, since maintaining the generator at its current state will be impossible once more features are added. My PR was blocked because of https://github.com/OpenAPITools/openapi-generator/pull/15830, which is now solved. So I can resume working on this starting tomorrow, and if it's ok with @wing328 can we postpone the 7.0.0 release just a little bit ?
this PR was pretty much done except for the handling of container variable types
I absolutely agree, I am just wondering if it is better to introduce a new experimental generator that can replace the old one in a later version. These changes are massive and I don't feel comfortable merging them into the current generator.
@kuhnroyal The thing is, I was planning on merging both the dart-dio
and dart
into just dart
.
so it will be already breaking for dart-dio users to migrate to the new dart generator.
and while a breaking change is annoying, its merits far outweigh the annoyances.
just look at the amount of issues dart
and dart-dio
currently have
most of them are easily fixable by unifying the codebase into serialization/api layers
But there is no need to break 2 generators and replace them with one that is mostly untested when the release comes. We can create a new generator and fix all the problems there, deprecate the old ones in a 7.1 release and then switch over in the next major release.
the main problem with that is:
UPDATE: I have changed the PR to introduce a new generator instead.
Is your feature request related to a problem? Please describe.
currently
dart-dio
json_serializable
: https://github.com/OpenAPITools/openapi-generator/tree/master/modules/openapi-generator/src/main/resources/dart/libraries/dio/serialization/json_serializable/apibuilt_value
: https://github.com/OpenAPITools/openapi-generator/tree/master/modules/openapi-generator/src/main/resources/dart/libraries/dio/serialization/built_value/apiDescribe the solution you'd like
We can mitigate both of these issues with some non-breaking refactor of the generated API classes, essentially removing that
api
mustache folder, that each serializer has to adapt.IJsonSerializationRepository
which all serialization options have to implement. It will include these 2 abstract methodswhere the general rule is that