swagger-api / swagger-api/swagger-codegen
[PHP] Response value can't be used without modification for 'format: date'
Nobody has claimed this yet.
- Dominant language
- Mustache
- Stars
- 17.8k
- Forks
- 6k
- PR merge metrics
- No merged PRs in 30d
Description
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 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).
Swagger declaration file content or url
test.yaml
---
swagger: '2.0'
info:
title: 'Test Codegen Date Parameter'
version: '1.0'
paths:
'/foo':
get:
parameters:
- name: date
in: query
type: string
format: 'date'
responses:
200:
description: 'Success'
schema:
type: object
properties:
date:
type: string
format: 'date'
client.php
<?php
require 'vendor/autoload.php';
use Swagger\Client\Api\DefaultApi;
$api = new DefaultApi(
new \GuzzleHttp\Client(),
(new \Swagger\Client\Configuration())->setHost('http://localhost')
);
$response = $api->fooGet();
$api->fooGet($response->getDate());
index.php
<?php
echo json_encode([ 'date' => date('Y-m-d') ]);
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.
- Save the sample
test.yamlin$PWD. - Create a directory
$PWD/fooand place in that directory the sampleindex.php. - In
$PWDrunphp -S 127.0.0.1:80to start a quick web server. - Generate the client library from
$PWD:java -jar swagger-codegen-cli-2.3.0-20170921.031930-142.jar generate -i test.yaml -l php - Go into the directory
$PWD/SwaggerClient-php. - Run
composer update --no-dev - Save the sample
client.phpin$PWD/SwaggerClient-php. - Run the client:
php client.php - Check the web server log, it will show:
[Thu Sep 21 09:43:01 2017] 127.0.0.1:56274 [200]: /foo
[Thu Sep 21 09:43:01 2017] 127.0.0.1:56276 [200]: /foo?date=2017-09-21T00%3A00%3A00%2B00%3A00
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
datea\DateTimebut makeObjectSerializeraware of the type of the parameter. The methodsanitizeForSerialization()already has this, if in the above sampleclient.phpyou add aprint_r((string)$response);you get the expected value:{ "date": "2017-09-21" }. If$typeand$formatare added as parameters to thetoPathValue(),toQueryValue(),toHeaderValue()andtoFormValue()methods it is possible to check for adateand format accordingly,sanitizeForSerialization()has:return ($format === 'date') ? $data->format('Y-m-d') : $data->format(\DateTime::ATOM);. -
Do not put a
datein a\DateTimeobject but keep it astring(as-is). A potential issue I see anyways with using a\DateTimeobject for adateis with time zones. By creating a\DateTimeyou are basically assuming that thedatein the response is for the same time zone as the client's server. A\DateTimethat 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-timehas, 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\DateTimeobjects of adateand adate-timeyou can run into unexpected results. By keeping adateas a string you put the burden of handling it correctly with the developer. They get to decide if they want to use a\DateTimeand if so how to handle the time zone. If the client generates a\DateTimewith 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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the PHP ObjectSerializer methods toString(), toPathValue(), toQueryValue(), toHeaderValue(), and toFormValue(), comparing them with sanitizeForSerialization(). Reproduce the generated client case from test.yaml and verify that passing the response date into the next request produces the expected /foo?date=YYYY-MM-DD URL.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100