Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • O openapi-generator
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 3,476
    • Issues 3,476
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 402
    • Merge requests 402
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • OpenAPI Tools
  • openapi-generator
  • Merge requests
  • !12532

[PHP] Enhance Symfony generator

  • Review changes

  • Download
  • Email patches
  • Plain diff
Merged Administrator requested to merge github/fork/dmetzner/enhanced-symfony-generator into master Jun 03, 2022
  • Overview 0
  • Commits 2
  • Pipelines 0
  • Changes 56

Created by: dmetzner

This merge request provides enhancements to the Symfony PHP generator all over the place by reducing possible syntax errors, stricter types, and "reworking" the default values feature. For more details check out the what has changed section.

These enhancements were added on the fly during the process of upgrading the Catrobat API to the new v6 generator. Therefore, this merge request contains quite a lot of changes for a single merge request. In case the merge request should be split into multiple smaller ones, please just say so :)

What has changed & and why?

  • Added missing type hints all over the place to: Stricter types allow to write a more secure code and detect possible issues with static analysis. For example, the Catrobat API now passes all phpstan checks up to level 4 without the necessity to manually fix some issues.

  • Added missing use statements. This was done to improve static analysis and prevent warnings.

  • Simplified model constructor Switched from ìsset` to the ternary operator Reason: Simpler code is better code -> fewer warnings in most IDEs.

  • Switched API controller $fallthrough Exception to Throwable Since more and more type hints have been introduced, the possibility to trigger type errors has also been introduced. This switch allows the runtime to catch such type errors & and send an error response without crashing.

  • void method result is no longer stored in a $result variable Improved controller API handling. In case a method returns nothing, nothing can't be stored in the $result variable. Hence, the result variable has no purpose and is just code smell.

  • changed the validate method By changing the logic to build the error string in the validate method, the method no longer has to force a string cast on the object, which could produce an unexpected result and hence is also a warning in most static analyzers.

  • enhancing the deserializeString method to support bool values, without the necessity to transform a boolean string to a boolean. This allows setting default request values already to a boolean value. The same logic was already implemented for integers.

  • Added default null values to the model properties. This allows accessing an uninitialized model's property without triggering a Throwable. Basically, resulting in the same logic which was already implemented in the model's constructor. However, this fixes the issue when no constructor is used to instantiate the model, for example, when a request has not the property. Since the object creation in the deserialization process is not using the constructor the property remained uninitialized and crashed during access in the getter. In addition, it was not possible to catch this Throwable with methods such as isset or empty, because those methods can only be used directly on the property and not a getter. Hence another fix to this issue could have been to get rid of those getters and setters and make the property public.

  • Simplified the return types of the API interfaces. This fixes various bugs introduced with v6 and return type declarations.

    204 API will remain : void; Everything else is now typed as : array|object|null

    It's not perfect, however, an easy first fix for the following bugs:

    • where sometimes the return value was typed as : array|\array which is invalid
    • where null has to be returned; e.g: in case of an error, where no response model can/should be built This was not possible when the type was fixed to e.g.: : array|SuccessResponseModel
    • where the API can return different Models for different status codes: E.g: 422 ErrorResponseModel + 200 SuccesResponseModel This was not possible when the type was fixed to e.g: : array|SuccessResponseModel

    Note: In case the : array|object|null return type declaration is needed instead for a 204 status code, for example for a 422 error Response which has content, the API specification can be "tricked" by defining content for 204

    '204':
        content:
            application/json:
                schema:
                type: object
    '422':
        content:
            application/json:
                schema:
                type: object
  • Default values are now actually working. The current implementation of default values for request parameters has no useful effects, however, introduces deprecations warnings and requires manual work within the projects implementing the API to fix those issues.

    Php8.0: Deprecation warning optional before required param - Example: function getFoo(string $type = "useless", int &$responseCode, array $responseHeaders) Now we have an optional parameter($type) before a required parameter($responseCode) which is bad. Hence, the optional parameter is no longer optional, and therefore the default value can't be used for something other than documentation.

    By removing the default values from the method signature we can get rid of this deprecation warning. However, the defined default values in the specification would still have zero purposes, other than documentation. Therefore, default values have been added to the, in my opinion, correct spot. From now on the second parameter in the requests get-method is used, which does exactly what is needed for the default values to work as expected. Example: $limit = $request->query->get('limit', 20) Now the $limit is either set by the request or by the default value, and all method parameters contain the expected value.

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 (6.0.1) (patch release), 6.1.x (breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request. @jebentier (2017/07), @dkarlovi (2017/07), @mandrean (2017/08), @jfastnacht (2017/09), @ackintosh (2017/09) ❤️, @ybelenko (2018/07), @renepardon (2018/12), @BenjaminHae, @wing328
Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: github/fork/dmetzner/enhanced-symfony-generator