[BUG][dart-dio] optimize parameterToString to convert DateTime correctly

[ahmednfwela]

Description

Currently DateTime is being converted to string using normal .ToString()
Ref:

openapi-generator/modules/openapi-generator/src/main/resources/dart-dio/api_util.mustache

Lines 6 to 14 in c322084

	String parameterToString(Serializers serializers, dynamic value) {
	if (value == null) {
	return '';
	} else if (value is String || value is num) {
	return value.toString();
	} else {
	return json.encode(serializers.serialize(value));
	}
	}

instead, toIso8601String should be used

openapi-generator version

V5.0.0

Suggested fix:

  if (value == null) {
    return '';
  } else if (value is String || value is num) {
    return value.toString();
  } else if (value is DateTime) {
    return value.toIso8601String();
  } else {
    return json.encode(serializers.serialize(value));
  }

[kuhnroyal]

If it is a DateTime it should use the configured serializer that is also used for JSOn content. Is this not happening?

[ahmednfwela]

no, even bool isn't serialized correctly
it outputs a json object rather than a json value

[kuhnroyal]

Yea we should not serialize primitives with a serializer.

[kuhnroyal]

Can you please review and possibly test the PR

[kuhnroyal]

If no specific serializer is used, built_value will add a discriminator field and make it a JSON object.
The check for bool was missing.

[ahmednfwela]

I think the issue here is that built_value should have handled primitives better than this, serialization shouldn't be so over-complicated.
I think we should file an issue at their side to handle this while we temporarily fix it.

[kuhnroyal]

parameterToString is only used for form data, we always need a normal String representation. That is not really what built_value is meant to be used for. Maybe the problem is triggered by the json.encode call.

In any case, for DateTime it should have used the registered Iso8601DateTimeSerializer.

[kuhnroyal]

I have update the PR and added tests, can you check if that solves your issues?

[ahmednfwela]

I have been using my own templates, and I believe they solve most of the issues that I came across

https://github.com/Bdaya-Dev/Custom-Openapi-Templates

They have been tested on multiple specifications too

[kuhnroyal]

Ok so I have not thought about collections. Looking at the Dio FormData implementation makes a couple things clearer: https://github.com/flutterchina/dio/blob/master/dio/lib/src/utils.dart#L34

It seems your list.join(',') is some custom handling that FormData doesn't support by default?

[ahmednfwela]

the problem is the way backend handles requests, some want it to be path[index]=value, others path=value0,value1,value2
this is highly opinionated, so it depends on what I am currently using

[ahmednfwela]

same reason I introduced fileNamesHaveBrackets too
https://github.com/Bdaya-Dev/Custom-Openapi-Templates/blob/master/dart-dio/apilib.mustache#L21

[kuhnroyal]

I think #8372 could solve most of it. We can not currently support custom collection formats. I added tests to ensure collections and date time is correctly encoded in the Dio FormData.
Dio encodes lists with braces but without index path[]=value1&path[]=value2. If you need different encoding then you still need to adapt the templates according to your needs.

[kuhnroyal]

After adding more tests, I noticed that this is also a question of multipart/form-data vs application/x-www-form-urlencoded. We are using the same method to prepare the parameters but Dio encodes them differently.
What are your use cases? multipart/form-data or application/x-www-form-urlencoded?

[ahmednfwela]

most of them are multipart/form-data

[kuhnroyal]

Both should now work with collections and path[]=value1&path[]=value2 format.
We can not configure the collection format for application/x-www-form-urlencoded until cfug/dio#808 is released.

[kuhnroyal]

A way to handle collection formats has been added to dio 4.0.0. If you need different formats in one route you might want to comment on cfug/dio#799 and follow #6681

Otherwise I think this issue can be closed.

[kuhnroyal]

The application/x-www-form-urlencoded part should now be handled correctly by #9635