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] Fix parentSchema conditional causing parent call when parent isn't present #8705

Merged
merged 2 commits into from
Apr 28, 2021
Merged

Conversation

buttilda
Copy link
Contributor

@buttilda buttilda commented Feb 15, 2021

we cannot call parent:: if parent isn't present. Everywhere else in the schema we're checking for {{parentSchema}} but here we're checking for {{parent}} which is causing errors when parent is not present

@jebentier (2017/07), @dkarlovi (2017/07), @mandrean (2017/08), @jfastnacht (2017/09), @ackintosh (2017/09) ❤️, @ybelenko (2018/07), @renepardon (2018/12)

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master, 5.1.x, 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

we cannot call `parent::` if parent isn't present. Everywhere else in the schema we're checking for `{{parentSchema}}` but here we're checking for `{{parent}}` which is causing errors when parent is not present
@buttilda buttilda changed the title Fix parentSchema conditional [PHP] Fix parentSchema conditional Feb 15, 2021
@buttilda buttilda changed the title [PHP] Fix parentSchema conditional [PHP] Fix parentSchema conditional causing parent call when parent isn't present Feb 15, 2021
@wing328
Copy link
Member

wing328 commented Feb 22, 2021

diff --git a/samples/client/petstore/php/OpenAPIClient-php/lib/Model/NullableClass.php b/samples/client/petstore/php/OpenAPIClient-php/lib/Model/NullableClass.php
index 341980399ac..c5d857b1243 100644
--- a/samples/client/petstore/php/OpenAPIClient-php/lib/Model/NullableClass.php
+++ b/samples/client/petstore/php/OpenAPIClient-php/lib/Model/NullableClass.php
@@ -258,7 +258,7 @@ class NullableClass implements ModelInterface, ArrayAccess, \JsonSerializable
      */
     public function listInvalidProperties()
     {
-        $invalidProperties = parent::listInvalidProperties();
+        $invalidProperties = [];
 
         return $invalidProperties;
     }
Perform git status
On branch pull/8705
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   samples/client/petstore/php/OpenAPIClient-php/lib/Model/NullableClass.php

no changes added to commit (use "git add" and/or "git commit -a")

Can you please run the following the update the samples?

./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh

@buttilda
Copy link
Contributor Author

buttilda commented Mar 4, 2021

@wing328 done (I think), I don't get any updates when I run the commands you suggested. So I did the change manually.

@sserbin
Copy link

sserbin commented Apr 12, 2021

@wing328 apologize for bumping this, but is there anything else that needs to be done here? Or is it pending review from php committee?

@wing328
Copy link
Member

wing328 commented Apr 13, 2021

@sserbin @ganymedes01 do you mind sharing a spec (minimal) to repeat the issue (without this fix)?

@sserbin
Copy link

sserbin commented Apr 13, 2021

@wing328 the following spec:

openapi: 3.0.0
servers:
components:
  schemas:
    Foo:
      type: object
      additionalProperties: true
      properties:
        id:
          type: integer
          format: int64
          nullable: false

produces this lib/Model/Foo.php - https://gist.github.com/sserbin/f9041c2ebc1b1844a7a17e827d19e7e8#file-openapi-8705-foo-php-L191

I think it's the additionalProperties that causes the addition of the faulty parent:: call

In php 8 this procudes a fatal error: https://3v4l.org/nsQL3

@wing328
Copy link
Member

wing328 commented Apr 28, 2021

Travis CI failure not related to this PR.

Tested this PR locally and the result is good. Thanks for the fix.

@wing328 wing328 merged commit ad9e239 into OpenAPITools:master Apr 28, 2021
@wing328 wing328 added this to the 5.1.1 milestone Apr 30, 2021
@buttilda buttilda deleted the patch-1 branch October 15, 2021 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants