allow both readOnly and required for a given property

[ghost]

Hi! First off, thanks for sharing this lib, I think it's pretty cool :)

Summary

As the title says, I wish I'd be able to use both readOnly and required for a given property. The goal is the following :

1. Allow users to make post requests without specifying the readOnly property (e.g. the table id)
2. Keep the id property as required so that users know they will get this value in the response and so that I can validate the responses using Connexion.
   According to this issue, this feature is now part of the OpenAPI sepcification.

Setup

In order to test whether this library allowed this use case, I used your Connexion example and I slightly modified the Employee schema :

Employee:
      description: Person that works for a company.
      type: object
      x-tablename: employee
      properties:
        id:
          type: integer
          description: Unique identifier for the employee.
          example: 0
          x-primary-key: true
          x-autoincrement: true
          readOnly: true // <-- 1st modification
        name:
          type: string
          description: The name of the employee.
          example: David Andersson
          x-index: true
        division:
          type: string
          description: The part of the company the employee works in.
          example: Engineering
          x-index: true
        salary:
          type: number
          description: The amount of money the employee is paid.
          example: 1000000.00
      required:
        - id // <-- 2nd modification.
        - name
        - division

Result

With the 1st modification, as expected, the swagger UI is not expecting the id property for the POST requests anymore. Great !
However, we get the following error (full traceback) :

Traceback (most recent call last):
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/open_alchemy/utility_base/__init__.py", line 103, in construct_from_dict_init
    jsonschema.validate(instance=kwargs, schema=schema)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/jsonschema/validators.py", line 934, in validate
    raise error
jsonschema.exceptions.ValidationError: 'id' is a required property

Failed validating <OpenApiProperties.REQUIRED: 'required'> in schema:
    {'description': 'Person that works for a company.',
     <OpenApiProperties.PROPERTIES: 'properties'>: {'division': {'description': 'The '
                                                                                'part '
                                                                                'of '
                                                                                'the '
                                                                                'company '
                                                                                'the '
                                                                                'employee '
                                                                                'works '
                                                                                'in.',
                                                                 'type': 'string'},
                                                    'id': {'description': 'Unique '
                                                                          'identifier '
                                                                          'for '
                                                                          'the '
                                                                          'employee.',
                                                           'readOnly': True,
                                                           'type': 'integer'},
                                                    'name': {'description': 'The '
                                                                            'name '
                                                                            'of '
                                                                            'the '
                                                                            'employee.',
                                                             'type': 'string'},
                                                    'salary': {'description': 'The '
                                                                              'amount '
                                                                              'of '
                                                                              'money '
                                                                              'the '
                                                                              'employee '
                                                                              'is '
                                                                              'paid.',
                                                               'type': 'number'}},
     <OpenApiProperties.REQUIRED: 'required'>: ['name', 'division', 'id'],
     <OpenApiProperties.TYPE: 'type'>: 'object'}

On instance:
    {'division': 'Engineering',
     'name': 'David Andersson',
     'salary': 1000000.0}

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/flask/app.py", line 2447, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/flask/app.py", line 1952, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/flask/app.py", line 1821, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/flask/_compat.py", line 39, in reraise
    raise value
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/flask/app.py", line 1950, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/flask/app.py", line 1936, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/connexion/decorators/decorator.py", line 48, in wrapper
    response = function(request)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/connexion/decorators/uri_parsing.py", line 173, in wrapper
    response = function(request)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/connexion/decorators/validation.py", line 194, in wrapper
    response = function(request)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/connexion/decorators/parameter.py", line 126, in wrapper
    return function(**kwargs)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/api.py", line 18, in post
    employee = models.Employee.from_dict(**body)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/open_alchemy/utility_base/__init__.py", line 179, in from_dict
    init_dict = cls.construct_from_dict_init(**kwargs)
  File "/home/ycrouin/Desktop/OpenAlchemy-master/examples/app/venv/lib/python3.8/site-packages/open_alchemy/utility_base/__init__.py", line 105, in construct_from_dict_init
    raise exceptions.MalformedModelDictionaryError(
open_alchemy.exceptions.MalformedModelDictionaryError: The dictionary passed to from_dict is not a valid instance of the model schema. schema={<OpenApiProperties.TYPE: 'type'>: 'object', <OpenApiProperties.PROPERTIES: 'properties'>: {'id': {'type': 'integer', 'description': 'Unique identifier for the employee.', 'readOnly': True}, 'name': {'type': 'string', 'description': 'The name of the employee.'}, 'division': {'type': 'string', 'description': 'The part of the company the employee works in.'}, 'salary': {'type': 'number', 'description': 'The amount of money the employee is paid.'}}, <OpenApiProperties.REQUIRED: 'required'>: ['name', 'division', 'id'], 'description': 'Person that works for a company.'}, kwargs={'division': 'Engineering', 'name': 'David Andersson', 'salary': 1000000.0}

Basically, the JSON schema validator is complaining that the id property is missing while it's required.

Finally, when we remove the required property (2nd modification), it works fine. However, the schema is not really accurate anymore from the response point of view.

Question

Is there any way to get around this ?

[jdkandersson]

Hi,

Thanks, I'm glad you like the software!

Yes there was a misalignment here between OpenAPI and JSONSchema in the past. There has recently been a new version of OpenAPI that has removed readOnly and writeOnly and aligned the schemas part of the OpenAPI spec with JSONSchema. You can read more about it here https://github.com/OAI/OpenAPI-Specification/releases/tag/3.1.0-rc0. I'm interested in your thoughts, does this convince you to no longer use readOnly and writeOnly given that it has been removed in the latest version of OpenAPI? Or is that an important feature for you?

David

[ghost]

Ha I see. I wasn't aware of the alignment in 3.1. The alignment is a great feature that was needed but on the other hand, we've lost some useful features such as this one :/ I wish they wouldn't have drop features from OpenApi and instead add them to the JSON schema specification but I'm sure they had good reasons.

I think readOnly and writeOnly are still useful when the property is not required (for instance, the readOnly properties won't show up for POST request examples but will for GET ones in swagger ui). However, if the property is required, like in my example above, then it's a problem, especially from the output point of view, since the schema is not accurate anymore. I guess it might not be a big deal for some users, but I would like my specification to be accurate for both requests/responses.

Right now, I see two solutions :

1. Modify the schema (or something with a similar effect) before using the json schema validator in order to keep the readOnly/required feature of OpenApi, such as :

if openApiVersion < 3.1 && operation = WRITE && attribute.readOnly is True
then attribute.required = False

2. As I saw in this Stackoverflow answer, use different schemas for write/read requests :

{
    "definitions": {
        "common": {
            "type": "object",
            "properties": {
                "id": {
                    "type": "number",
                    "readOnly": true
                },
                "title": {
                    "type": "string"
                },
                "post": {
                    "type": "string"
                }
            },
            "required": ["title", "post"],
            "additionalProperties": false
        },
        "input": {
            "allOf": [
                {"$ref": "#/definitions/common"},
                {"properties": {"id": false}}
            ]
        },
        "output": {
            "allOf": [
                {"$ref": "#/definitions/common"},
                {"required": ["id"]}
            ]
        }
    }
}

I guess the 2nd solution is the most robust, since we don't tweak the schemas ourselves, but it's also much more verbose and this would create two SQLAlchemy models.

What do you think ?

[jdkandersson]

Would the following slight modification for the second approach work? Define 2 schemas, the input schema that does not include the id property but sets everything else to required, and define the output schema that inherits from the input schema and also defines the id property that is required. Then, to store everything in the same table, use SQLAlchemy single table inheritance (https://openapi-sqlalchemy.readthedocs.io/en/latest/technical_details/inheritance.html#single-table-inheritance).

[ghost]

Hi, thanks for the suggestion ! Here's what I tried to do :

components:
  schemas:
    EmployeeInput:
      description: Person that works for a company.
      type: object
      x-tablename: employee
      properties:
        id:
          type: integer
          description: Unique identifier for the employee.
          example: 0
          x-primary-key: true
          x-autoincrement: true
          readOnly: true
        name:
          type: string
          description: The name of the employee.
          example: David Andersson
          x-index: true
        division:
          type: string
          description: The part of the company the employee works in.
          example: Engineering
          x-index: true
        salary:
          type: number
          description: The amount of money the employee is paid.
          example: 1000000.00
      required:
        - name
        - division

    EmployeeOutput:
      allOf:
        - $ref: "#/components/schemas/EmployeeInput"
        - x-inherits: true
          x-kwargs:
            __mapper_args__:
              required:
                - id

I'm not sure whether my code is incorrect, maybe I did something wrong. I'm having the following error :

open_alchemy.exceptions.MalformedSchemaError: EmployeeOutput :: properties :: models must have at least 1 property themself

In the end, since I have very limited time to work on my POC, here's what I did for now :

• I removed the readOnly properties from the required properties to generate the models
• Once the models are generated, I did put the properties back to required and I don't re-generate the models until I change the OpenApi spec.

You can close this issue if you want.

[jdkandersson]

Closing for now. If you would like to fix the issue above, it is because each model needs to define at least 1 property itself. So if id is not required for EmployeeInput, don't define it there but instead define it on EmployeeOutput. You would also need a property to differentiate between the two models, so it isn't a perfect solution either.