You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
If you have a response value set to format: date the PHP library will turn that into a \DateTime object. However a parameter that is passed as a \DateTime object is always formatted as \DateTime::ATOM in ObjectSerializer->toString().
This means that when you get a response value of format: date and want to pass that as a parameter in a next request, you can't pass it directly but have to pass it as $value->format('Y-m-d'). This feels counter-intuitive, I would expect if a response and a parameter are of the same type and format in the definitions, they can be passed around as-is.
Swagger-codegen version
I'm using the latest 2.3.0 snapshot (swagger-codegen-cli-2.3.0-20170921.031930-142.jar).
The URL that should have been called is /foo?date=2017-09-21.
Related issues/PRs
I came across this after further testing with #6474, but the issue and PR itself are unrelated to this problem, hence the new issue.
Suggest a fix/enhancement
I think there are two ways to make the behavior more intuitive:
Keep a date a \DateTime but make ObjectSerializer aware of the type of the parameter. The method sanitizeForSerialization() already has this, if in the above sample client.php you add a print_r((string)$response); you get the expected value: { "date": "2017-09-21" }. If $type and $format are added as parameters to the toPathValue(), toQueryValue(), toHeaderValue() and toFormValue() methods it is possible to check for a date and format accordingly, sanitizeForSerialization() has: return ($format === 'date') ? $data->format('Y-m-d') : $data->format(\DateTime::ATOM);.
Do not put a date in a \DateTime object but keep it a string (as-is). A potential issue I see anyways with using a \DateTime object for a date is with time zones. By creating a \DateTime you are basically assuming that the date in the response is for the same time zone as the client's server. A \DateTime that gets created from only a date is still a date and time, the time is just 00:00:00 in the default time zone (or if a time zone was specific, 00:00:00 in the specified time zone). However anything formatted as date-time has, per the Swagger/OpenAPI specifications, a time zone associated with it which is not necessarily the same as the default of the client's server. When you then start comparing the \DateTime objects of a date and a date-time you can run into unexpected results. By keeping a date as a string you put the burden of handling it correctly with the developer. They get to decide if they want to use a \DateTime and if so how to handle the time zone. If the client generates a \DateTime with an incorrect time zone you have to do some ugly stuff like new \DateTime($date->format('Y-m-d'), new DateTimeZone('EST')) to get a proper object, because you can't call setTimezone(), that potentially changes the date in the object.
My preferred solution is the second option, it should be the developer's responsibility to figure out how to handle dates and not have the client make possible wrong assumptions.
The text was updated successfully, but these errors were encountered:
Description
If you have a response value set to
format: date
the PHP library will turn that into a\DateTime
object. However a parameter that is passed as a\DateTime
object is always formatted as\DateTime::ATOM
inObjectSerializer->toString()
.This means that when you get a response value of
format: date
and want to pass that as a parameter in a next request, you can't pass it directly but have to pass it as$value->format('Y-m-d')
. This feels counter-intuitive, I would expect if a response and a parameter are of the same type and format in the definitions, they can be passed around as-is.Swagger-codegen version
I'm using the latest 2.3.0 snapshot (
swagger-codegen-cli-2.3.0-20170921.031930-142.jar
).Swagger declaration file content or url
test.yaml
client.php
index.php
Command line used for generation
java -jar swagger-codegen-cli-2.3.0-20170921.031930-142.jar generate -i test.yaml -l php
Steps to reproduce
I am assuming
$PWD
is your current working directory where you are testing this. Replace of course as you like with a directory of your choice.test.yaml
in$PWD
.$PWD/foo
and place in that directory the sampleindex.php
.$PWD
runphp -S 127.0.0.1:80
to start a quick web server.$PWD
:java -jar swagger-codegen-cli-2.3.0-20170921.031930-142.jar generate -i test.yaml -l php
$PWD/SwaggerClient-php
.composer update --no-dev
client.php
in$PWD/SwaggerClient-php
.php client.php
The URL that should have been called is
/foo?date=2017-09-21
.Related issues/PRs
I came across this after further testing with #6474, but the issue and PR itself are unrelated to this problem, hence the new issue.
Suggest a fix/enhancement
I think there are two ways to make the behavior more intuitive:
Keep a
date
a\DateTime
but makeObjectSerializer
aware of the type of the parameter. The methodsanitizeForSerialization()
already has this, if in the above sampleclient.php
you add aprint_r((string)$response);
you get the expected value:{ "date": "2017-09-21" }
. If$type
and$format
are added as parameters to thetoPathValue()
,toQueryValue()
,toHeaderValue()
andtoFormValue()
methods it is possible to check for adate
and format accordingly,sanitizeForSerialization()
has:return ($format === 'date') ? $data->format('Y-m-d') : $data->format(\DateTime::ATOM);
.Do not put a
date
in a\DateTime
object but keep it astring
(as-is). A potential issue I see anyways with using a\DateTime
object for adate
is with time zones. By creating a\DateTime
you are basically assuming that thedate
in the response is for the same time zone as the client's server. A\DateTime
that gets created from only a date is still a date and time, the time is just 00:00:00 in the default time zone (or if a time zone was specific, 00:00:00 in the specified time zone). However anything formatted asdate-time
has, per the Swagger/OpenAPI specifications, a time zone associated with it which is not necessarily the same as the default of the client's server. When you then start comparing the\DateTime
objects of adate
and adate-time
you can run into unexpected results. By keeping adate
as a string you put the burden of handling it correctly with the developer. They get to decide if they want to use a\DateTime
and if so how to handle the time zone. If the client generates a\DateTime
with an incorrect time zone you have to do some ugly stuff likenew \DateTime($date->format('Y-m-d'), new DateTimeZone('EST'))
to get a proper object, because you can't callsetTimezone()
, that potentially changes the date in the object.My preferred solution is the second option, it should be the developer's responsibility to figure out how to handle dates and not have the client make possible wrong assumptions.
The text was updated successfully, but these errors were encountered: