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

[BUG] [PHP] generateAliasAsModel leads to invalid parameter types on models. #8945

Closed
5 of 6 tasks
neclimdul opened this issue Mar 10, 2021 · 2 comments · Fixed by #10093
Closed
5 of 6 tasks

[BUG] [PHP] generateAliasAsModel leads to invalid parameter types on models. #8945

neclimdul opened this issue Mar 10, 2021 · 2 comments · Fixed by #10093

Comments

@neclimdul
Copy link
Contributor

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

Documented types are incorrect when a model property is a list of another model and the child model is an "alias" of array.

I think the php generator is treating the model as a model regardless and prefixing the namespace but maybe but some other core part of the generator is collapsing it to an array leaving this invalid type? I thought I had a theory from the name generation and the php generator not listing array as a primitive but that seems to not cause problems. At least I can rationalize exactly where the bug would be.

Then I noticed generateAliasAsModel being mentioned in verbose output. I can confirm that adding --generate-alias-as-model fixed the type.

openapi-generator version

tested with 5.

OpenAPI declaration file content or url
swagger: '2.0'
info:
  description: Marketo Rest API
  version: '1.0'
  title: Marketo Rest API
  termsOfService: 'https://www.marketo.com/company/legal/'
  contact:
    name: Marketo Developer Relations
    url: 'http://developers.marketo.com'
    email: developerfeedback@marketo.com
  license:
    name: API License Agreement
    url: 'http://developers.marketo.com/api-license/'
host: 'localhost:8080'
basePath: /
schemes:
  - https
tags:
  - name: Leads
    description: Leads Controller
paths:
  /rest/v1/leads/describe2.json:
    get:
      tags:
        - Leads
      summary: Describe Lead2
      description: 'Returns list of searchable fields on lead objects in the target instance.  Required Permissions: Read-Only Lead, Read-Write Lead'
      operationId: describeUsingGET_6
      consumes:
        - application/json
      produces:
        - application/json
      responses:
        '200':
          description: OK
          schema:
            $ref: '#/definitions/LeadAttribute2'
definitions:
  LeadAttribute2:
    type: object
    required:
      - name
      - searchableFields
      - fields
    properties:
      name:
        type: string
        description: '"API Lead"'
      searchableFields:
        type: array
        description: List of searchable fields
        items:
          $ref: '#/definitions/LeadAttribute2SearchableFields'
      fields:
        type: array
        description: Description of searchable fields
        items:
          $ref: '#/definitions/LeadAttribute2Fields'
  LeadAttribute2SearchableFields:
    type: array
    description: List of searchable fields
    items:
      type: string
      description: Searchable field
  LeadAttribute2Fields:
    type: object
    required:
      - name
    properties:
      name:
        type: string
        description: REST API name of field
Generation Details

docker run --rm -v $PWD:/source -v ${PWD}/tmp:/output/ openapitools/openapi-generator-cli:v5.0.1 generate -i test.yaml -g php -o /output

Steps to reproduce

Running the generator, the LeadAttribute2.php model ends up with the following incorrectly typed code:

<?php
     * Gets searchable_fields
     *
     * @return \OpenAPI\Client\Model\array[]
     */
    public function getSearchableFields()
    {
        return $this->container['searchable_fields'];
    }

    /**
     * Sets searchable_fields
     *
     * @param \OpenAPI\Client\Model\array[] $searchable_fields List of searchable fields
     *
     * @return self
     */
    public function setSearchableFields($searchable_fields)
    {
        $this->container['searchable_fields'] = $searchable_fields;

        return $this;
    }
Related issues/PRs

generateAliasAsModel does have some other bugs in the queue that seem similar but I didn't see one that quite matched this. I could be wrong though.

Suggest a fix

abstract php generator might need to take into account the generateAliasAsModel option when prepending namespace somewhere? I'm not sure how to test this so I'm not sure how to narrow down the problem.

@neclimdul
Copy link
Contributor Author

Oh hi past me.

This isn't just a documentation problem as might be suggested by the summary, the "fix" of turning on the model aliases actually trades one problem for a different one because this is all tied to serialization. Now the type is "valid" but the serialization has to deal with the fact that we're converting an array into an object and it doesn't know how to do it. Probably one of the related bugs I mentioned.

I've pulled up the generator code and I still can't rationalize where this is a bug with the PHP codegen specifically. The AbstractPhpCodegen doesn't do anything with the alias option so the fact that its triggering the problem suggests to me its somewhere else.

@neclimdul
Copy link
Contributor Author

After a lot of tracing and digging and writing tests I'm fairly confident that fix is pretty straight forward. The PHP languageSpecificPrimitives doesn't include array which seems to trigger a slew of problems.

  1. The dataType doesn't seem to get populated correctly.
  2. The template can't properly detect that the type is a primitive.
    You can see in Make php's array primitive #10093 this was actually creating bad docs in the sample with things like [**float[][]**](array.md) linking to no-existent documentation.

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

Successfully merging a pull request may close this issue.

1 participant