Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PHP] Response value can't be used without modification for 'format: date' #6533

Open
SunMar opened this issue Sep 21, 2017 · 1 comment
Open

Comments

@SunMar
Copy link

SunMar commented Sep 21, 2017

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.

  1. Save the sample test.yaml in $PWD.
  2. Create a directory $PWD/foo and place in that directory the sample index.php.
  3. In $PWD run php -S 127.0.0.1:80 to start a quick web server.
  4. Generate the client library from $PWD: java -jar swagger-codegen-cli-2.3.0-20170921.031930-142.jar generate -i test.yaml -l php
  5. Go into the directory $PWD/SwaggerClient-php.
  6. Run composer update --no-dev
  7. Save the sample client.php in $PWD/SwaggerClient-php.
  8. Run the client: php client.php
  9. 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:

  1. 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);.

  2. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants