Add polymorphism in the PHP client

[arnested]

This pull request adds polymorphism in the PHP clients model part.

Models with an allOf definition will now be generated as proper class inheritance.

Because I needed to separate the $swaggerTypes, $attributeMap, $setters, and $getters static properties into the parent class and subclass I had to introduce static methods for accessing the complete array of those values instead of accessing them directly. This change uses PHP features only present in PHP 5.4 and later (I don't know what version is currently supported but PHP 5.3 is deprecated anyway).

I also tried to introduce polymorphism in the PHP client api part but I'm stuck on how to access the value of the discriminator attribute in DefaultCodengen.javas fromOperation()method. Have a look at this code patch: https://github.com/arnested/swagger-codegen/commit/c8a6129eb62c898b347d5d20aecf2bc1557ece20. Any advice would be appreciated!

[wing328]

@arnested thanks for the PR :)

Do you mind running ./bin/php-petstore.sh to update the PHP petstore sample and run the unit test as described here: https://github.com/swagger-api/swagger-codegen/tree/master/samples/client/petstore/php/SwaggerClient-php#tests to ensure the unit testing is ok?

(I'll include the PHP unit test as part of the CI later...)

[arnested]

I have pushed a regenerated version of the PHP petstore sample (SHA). There are also some other changes though - probably because the sample hasn't been regenerated in a while.

I have rebased the pull request against latest master.

I hadn't noticed the phpunit tests before. Thank you for pointing me towards them.

The tests throw one error - but that error was also present prior to the pull request (I tested to be sure):

PHPUnit 4.8.16 by Sebastian Bergmann and contributors.

.........E...

Time: 5.56 seconds, Memory: 8.00Mb

There was 1 error:

1) PetApiTest::testUploadFile
curl_setopt(): The usage of the @filename API for file uploading is deprecated. Please use the CURLFile class instead

/Users/arne/Documents/Kitematic/noex/www/private/api/swagger-codegen/samples/client/petstore/php/SwaggerClient-php/lib/ApiClient.php:174
/Users/arne/Documents/Kitematic/noex/www/private/api/swagger-codegen/samples/client/petstore/php/SwaggerClient-php/lib/Api/PetAPI.php:692
/Users/arne/Documents/Kitematic/noex/www/private/api/swagger-codegen/samples/client/petstore/php/SwaggerClient-php/tests/PetApiTest.php:196

FAILURES!
Tests: 13, Assertions: 601, Errors: 1.

[arnested]

I submitted a fix for the test case error in #1451.

[arnested]

The branch has been rebased on current master.

Besides I managed to get polymorphism into the PHP client API part. The client API will now be able handle responses in all subclasses of a model and use the models discriminator attribute to create the right subclass.

The client API part requires a snapshot release of Swagger Parser because it contained a bug.

Because of that I haven't merged the client API into this pull request. Instead it is located in a separate branch (based on top of this pull request). See the additional changes here: https://github.com/arnested/swagger-codegen/compare/php-polymorphism-model...arnested:php-polymorphism-client.

Feedback on all this would be appreciated!

[arnested]

Just a short reminder for when I look into this again.

The discriminator property should probably/maybe not have a setter method and the getter method should probably/maybe just return the class name (via reflection or just hardcoded via the mustache templates).

[emrahs]

Hey folks! Is there any update on this? Should I expect this to be merged soon? Please let me know if there's anything I can help with to get this merged. Thanks!

[fehguy]

@wing328 did this look OK to you?

[arnested]

Give me a couple of days to bring this up to date. I'll probably find time during the weekend.

[wing328]

Thanks @arnested

You may want to leverage the test spec (modules/swagger-codegen/src/test/resources/2_0/discriminatorTest.json) added via #2202 for discriminator support by updating the test case with it: https://github.com/swagger-api/swagger-codegen/pull/2202/files#diff-a5d745502133504837979497063fbfc1R47

[arnested]

Thank you for the pointer to #2202, @wing328. I hadn't noticed that one.

I just pushed my current implementation to this pull request.

My implementation includes inheritance in the PHP client models and adds polymorphism to the PHP client as well.

My work also includes adding support for the discriminator but I will have to look closer into the work of #2202 to see whether I could use that implementation instead or if they don't touch the same area.

My current version is rebased on master and so includes the change from #2202. At least it doesn't break my use cases with both implementations present at the same time.

The work I just pushed to this pull request is what we are actually using to generate a PHP API client for our current project (launched to production two weeks ago).

[arnested]

I had a look into #2202 and it implements part of what I was implementing in DefaultCodegen.java. #2202 did it in a much simpler fashion though. Thank you, @traviscollins.

I adapted my implementation to take advantage of #2202.

So what does this pull request really do now?

• It adds the discriminator to the Codegen Operation object.
• Changes the PHP client:
  • Uses inheritance in models (class A extends B instead of inlining B in A)
  • Adds polymorphism in the PHP client (uses the discriminator to serialize objects into the right (sub)class).
  • Regenerates PHP petstore sample.

We have basically been using this implementation since November and I think it is ready to be merged.

@emrahs, if you still have time to help out it would be nice if you could take this for spin and report back.

I ran the phpunit test-suite and it succeeds.

[arnested]

And I just added a test case (modules/swagger-codegen/src/test/java/io/swagger/codegen/CodegenTest.java) testing the presence of the discriminator in the Codegen Operation object using modules/swagger-codegen/src/test/resources/2_0/discriminatorTest.json from #2202.

[wing328]

@arnested thanks for the work and adding an test case in CodegenTest.java. I did some tests and the output looks good.