[BUG][python] Cannot instantiate class without required read-only parameters for POST endpoint

[gbmarc1]

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)

@spacether

Python generator

Description

I have a POST endpoint and the request body and response body use the same schema definition as below. Dataset has multiple readOnly parameters that must be present in the response, but should not be provided in the request.

I can instantiate the Dataset model providing None to the parameters that are readOnly and without verifying the model's input (_check_type=False). However, post request still fails (404 bad request) because params are still present with None values.

openapi-generator version

5.1.1

OpenAPI declaration file content or url

paths:
  /datasets:
    post:
      operationId: v1_inference_datasets_post
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Dataset'
        description: The dataset to be registered
        required: true
      responses:
        '201':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Dataset'
          description: Confirmation of dataset registration
      summary: Register a dataset
      tags:
        - datasets
 
components:
  schemas:
    Dataset:
      additionalProperties: false
      example:
        statusLastChange: 2000-01-23T04:56:07+00:00
        modifiedOn: 2000-01-23T04:56:07+00:00
        name: name
        inactiveOn: 2000-01-23T04:56:07+00:00
        description: description
        mediaType: text/plain
        id: 0
        createdOn: 2000-01-23T04:56:07+00:00
        status: active
      properties:
        id:
          description: ID of the dataset
          readOnly: true
          type: integer
        name:
          description: User-defined name of the dataset
          maxLength: 128
          type: string
        description:
          description: User-defined description of the dataset
          maxLength: 256
          type: string
        mediaType:
          description: Data type of the dataset. All data must be of the same type.
          enum:
            - text/plain
          type: string
        createdOn:
          description: Creation time of the dataset
          format: date-time
          readOnly: true
          type: string
        modifiedOn:
          description: Modification time of the dataset
          format: date-time
          readOnly: true
          type: string
        inactiveOn:
          description: |
            Time when the dataset will become inactive. Make sure to retrieve all data and job results
            before this time.
          format: date-time
          readOnly: true
          type: string
        status:
          description: |
            Status of the dataset:
              * `active`: The dataset can be modified (including the data it contains).
              * `committed`: The dataset can no longer be
                 modified (including the data it contains) but jobs can be submitted.
              * `inactive`: The dataset can no longer be modified (including the
                data it contains) and jobs cannot be submitted. Once a dataset is inactive,
                it becomes a candidate for deletion meaning it will soon be deleted from the system.
          enum:
            - active
            - committed
            - inactive
          readOnly: true
          type: string
        statusLastChange:
          description: Time when the status last changed
          format: date-time
          readOnly: true
          type: string
      required:
        - createdOn
        - description
        - id
        - inactiveOn
        - mediaType
        - modifiedOn
        - name
        - status
        - statusLastChange
      type: object

Generation Details

generate -i openapi/openapi.yaml -g "python" --package-name ex_ai_hub_api_client2 -p packageVersion=0.2.0

Steps to reproduce

Related issues/PRs

Suggest a fix

I suggest the following 3 modifications:

1. Modify the signature of __init__ for readOnly=True parameters from

Dataset:
    @convert_js_args_to_python_args
    def __init__(self, id, name, description, created_on, modified_on, inactive_on, status, status_last_change, *args, **kwargs):

to

Dataset:
    @convert_js_args_to_python_args
    def __init__(self, id, name, description, created_on=None, modified_on=None, inactive_on=None, status=None, status_last_change=None, *args, **kwargs):

2. Modify the checks accordingly to support null values when instantiating a class
3. Do not add parameter to body payload for readOnly=True and nullable=False parameters when value is None .

[spacether]

None should not be used as a default value because any property required or optional may or may not be Nullable.

[gbmarc1]

None should not be used as a default value because any property required or optional may or may not be Nullable.

What do you think of a new class? Such as Empty

class Empty:
    pass


@convert_js_args_to_python_args
def __init__(self, name, description, id=Empty(), created_on=Empty(), modified_on=Empty(), inactive_on=Empty(), status=Empty(), status_last_change=Empty(), *args, **kwargs):

[spacether]

Including a placeholder value like that could work but

• instantiation could would need to be tweaked to handle those values depending on the context in which we are creating our instance to_server vs from_server
• doing it this way makes it unclear what context we are in when instantiating the model. When done this way, our method signature combines both contexts. New devs reading the code will have questions, when is Empty used? What impact does it have on required/optional? How does it work when payloads are from the server vs to the server?

readOnly functionality is dependent on the context in which the payload is instantiated.

• fromServer: readonly attributes may be written to
• toServer: readonly attributes should be omitted (fromClient)
  Per the 3.0.2 spec:
  the property is marked as readOnly being true and is in the required list, the required will take effect on the response only. A property MUST NOT be marked as both readOnly and writeOnly being true. Default value is false.

That wording means that instantiating the models in different contexts needs different method signatures.

• one creation method for the to_server context where read_only params are omitted from the method signature
• one creation method for the from_server context where read_only params are included in the method signature

So my suggestion would be to break out a separate method to instantiate classes in the two contexts.

• __init__ could be used for the toServer context
• Another method could be used for the from_server context, maybe named from_openapi_data

We already use the parameter spec_property_naming to indicate that the code is in the context of from_server

[gbmarc1]

I think you are right. The init constructor should be used for the to_server context and another constructor for the from_server. I was looking for another feature "deserialization" in the models to convert a dict to an instance of the model. I suggest to name the method "from_dict". A bit like this:

openapi-generator/modules/openapi-generator/src/main/resources/python-flask/model.mustache

Line 64 in 22950fa

@classmethod

[spacether]

Model instantiation is not only done with dictionaries.
We also have array models and primitive model.
Primitive models are made if there are validation or enum constraints.
So we are instantiating models from openapi data, not just dicts.

Some examples:

• number with validations: https://github.com/OpenAPITools/openapi-generator/blob/master/samples/openapi3/client/petstore/python/petstore_api/model/number_with_validations.py#L30
• array containing enums: https://github.com/OpenAPITools/openapi-generator/blob/master/samples/openapi3/client/petstore/python/petstore_api/model/array_of_enums.py#L34

We need all of those model types because endpoints can return refed component schemas for:

• primitive types that have validation and enum constraints on them (inst, str, float, number, bool, date, datetime)
• list types with validations at the root level (min or max number of items)
• object types with root level constraints or constraints in property schemas

If we did not make the list and primitive model types, then we would leave out the enum and validation constraints.

[gbmarc1]

ok sounds a good plan. I can try to implement that next week

[spacether]

Thank you

[gbmarc1]

Could I have a reference that explain what is the difference between normal/composed/simple models?

[spacether]

ModelNormal models are classes that store object schemas
ModelSimple models are classes that store int/str/list/date/datetime schemas and they are only generated if there is a validation or enum constraint or if the model is a list model
ModelComposed models are classes that store composed schemas
Also described in code in the class definitions:
https://github.com/OpenAPITools/openapi-generator/blob/master/samples/openapi3/client/petstore/python/petstore_api/model_utils.py#L288

Composed schemas are not always of type object.
They can be any type. For example oneOf different array definitions.

[spacether]

Adding the classification Enhancement: Feature because the feature readOnly parameters has not yet been implemented in python