Improve model validation of returned object

[tschaffter]

[boyleconnor]

Will fill in details soon; this is the result of this bug in openapi-generator-cli.

[tschaffter]

@cascadianblue Revisiting this issue. I really like the idea that the default code validates the output returned by the tool, which would be useful feedback to the developer. This comes at the cost of using a custom template for the generator. The one-line command show in the past to generate a tool stub would have one more argument which is basically the path to the folder that includes the templates. For easy access, we would need to move these template to GitHub Pages. Not clear where these template should live as there will be different template for different tools. Should they live in the repository that include the example tool we provide for a given tool? Or the schema repo? Or in a dedicated repository?

[tschaffter]

I can already create the template and place them in this repo. We can then decide later where they will ultimately live.

[tschaffter]

@cascadianblue I tried applying the fix that we proposed to better validate models but I can't get it to work like last time (see #26). The solution we proposed was to update the constructor of the models so that model parameter setter methods are called, which is where validation of parameter value is done. However, doing that now leads to an error when a controller deserialize a JSON payload. The issue is in this utility function:

def deserialize_model(data, klass):
    """Deserializes list or dict to model.

    :param data: dict, list.
    :type data: dict | list
    :param klass: class literal.
    :return: model object.
    """
    instance = klass()

    if not instance.openapi_types:
        return data

    for attr, attr_type in six.iteritems(instance.openapi_types):
        if data is not None \
                and instance.attribute_map[attr] in data \
                and isinstance(data, (list, dict)):
            value = data[instance.attribute_map[attr]]
            setattr(instance, attr, _deserialize(value, attr_type))

    return instance

The instruction instance = klass() actually creates an empty model - without parameters - which triggers the validation of the value and throw an error if we apply the changes we proposed. I don't see a solution to this so I propose to shelf this shelve this work.

[mggevaer]

@tschaffter
I wanted to rely on Openapi generator to validate incoming objects, the solution I currently use is to use a utility method that calls all the setters on an object using its own values.
Maybe this is a low complexity/maintenance solution which can be included?

@something.post("/endpoint")
def post_something(user_id):
    request_body = SomethingRequestBody.from_dict(request.json)
    validate_object_using_setters(request_body) # This calls all the setters using the request body

def validate_object_using_setters(obj):
    """
    Calls all the possible setters of an object using its own values. Goes through nested objects as well.
    """
    items = vars(obj).items()
    obj = copy.deepcopy(obj)
    for property_name, value in items:
        if property_name.startswith('_'):
            property_name = property_name[1:]
        setattr(obj, property_name, value)

[tschaffter]

Thanks for reaching out @MartijnGevaert ! The validation of incoming objects should work out of the box.

For example, consider this POST function that I modified to show that we are not doing the validation ourselves.

def create_text_person_name_annotations():  # noqa: E501
    """Annotate person names in a clinical note

    Return the person name annotations found in a clinical note # noqa: E501

    :rtype: TextPersonNameAnnotationResponse
    """
    res = TextPersonNameAnnotationResponse([])
    status = 200
    return res, status

When submitting an object that is invalid, we get the following error:

http://localhost:8080/api/v1/textPersonNameAnnotations validation error: 12 is not of type 'string' - 'note.text'
127.0.0.1 - - [29/Jan/2022 14:15:34] "POST /api/v1/textPersonNameAnnotations HTTP/1.1" 400 -

The incoming payloads are validated before the function create_text_person_name_annotations() is even called. This GH ticket was about validating returned objects.

EDIT: The validation is performed using code that has been automatically generated by the OpenAPI generator.

• Deserialization utility function
• Model with validation

[mggevaer]

Hi @tschaffter
It seems that the json validation is not happening automatically in my project for INCOMING, REQUEST bodies, so for example the code below works fine if I don't pass along any body whatsoever, even though it should be

{
   "answer": "some string which is not None with at least length 1"
}

@somethign.post(/some-endpoint")
def post_something():
   # I'm passing None to the from_dict method to emulate an incoming post without a body.
    request_body = SomeRequestBody.from_dict(None)  

The way I make this fail is by using my validation util method.

Here is the SomeRequestBody code to prove that it should fail. I changed some names as this is part of private code, so don't worry if you see some inconsistencies.

class SomeRequestBody(Model):
    """NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).

    Do not edit the class manually.
    """

    def __init__(self, answer=None):  # noqa: E501
        """SomeRequestBody- a model defined in OpenAPI

        :param answer: The answer of this SomeRequestBody.  # noqa: E501
        :type answer: str
        """
        self.openapi_types = {
            'answer': str
        }

        self.attribute_map = {
            'answer': 'answer'
        }

        self._answer = answer

    @classmethod
    def from_dict(cls, dikt) -> 'SomeRequestBody':
        """Returns the dict as a model

        :param dikt: A dict.
        :type: dict
        :return: The SomeRequestBodyof this SomeRequestBody.  # noqa: E501
        :rtype: SomeRequestBody
        """
        return util.deserialize_model(dikt, cls)

    @property
    def answer(self):
        """Gets the answer of this SomeRequestBody.


        :return: The answer of this SomeRequestBody.
        :rtype: str
        """
        return self._answer

    @answer.setter
    def answer(self, answer):
        """Sets the answer of this SomeRequestBody.


        :param answer: The answer of this SomeRequestBody.
        :type answer: str
        """
        if answer is None:
            raise ValueError("Invalid value for `answer`, must not be `None`")  # noqa: E501
        if answer is not None and len(answer) < 1:
            raise ValueError("Invalid value for `answer`, length must be greater than or equal to `1`")  # noqa: E501

        self._answer = answer

I also updated my validate_object_using_setters code quite a bit, looks something like this now:

import copy
from collections.abc import Collection
from typing import Any


class SetterBasedObjectValidator:
    """
    Validates objects by calling all the possible setters of an object using its own values. Goes through nested objects and lists as well.
    """

    @staticmethod
    def validate(item: Any) -> None:
        if SetterBasedObjectValidator._has_attributes(item):
            SetterBasedObjectValidator._validate_dict_object_using_setters(item)
        elif SetterBasedObjectValidator._is_collection_but_not_str(item):
            SetterBasedObjectValidator._validate_objects_in_collection(item)

    @classmethod
    def _has_attributes(cls, obj: Any):
        """
        Items such as a str, numbers, list don't have user defined class attributes.
        """
        return hasattr(obj, '__dict__')

    @classmethod
    def _validate_dict_object_using_setters(cls, obj: dict) -> None:
        """
        Remember that all objects with fields are dicts according to python
        """
        items = vars(obj).items()
        obj = copy.deepcopy(obj)
        for property_name, value in items:
            if property_name.startswith('_'):
                property_name = property_name[1:]
            setattr(obj, property_name, value)
            cls.validate(value)

    @classmethod
    def _is_collection_but_not_str(cls, obj: Any):
        return isinstance(obj, Collection) and not isinstance(obj, (str, bytes, bytearray))

    @classmethod
    def _validate_objects_in_collection(cls, collection: Collection):
        """Can't handle strings"""
        for item in collection:
            cls.validate(item)

[tschaffter]

Your POST function in Python looks different from the ones I generated for this repo with OpenAPI generator (see this example). What version of the OpenAPI generator did you use? Did you use the generator python-flask?

[mggevaer]

version 5.4, python-flask yes, using this command:
java -jar ./openapi-generator-cli.jar generate -i ./backend_openapi_spec.yaml -o ../flask_server_stub -g python-flask

I'm only using the models from openapi though, not any of the generated controller files.
Reason being that I don't quite understand how you can make those controllers work with your custom code, you can't regenerate them without losing your custom code right?
And if you only generate them once, they go out of sync with the openapi, but might give the impression that they are still up to date with openapi.
(not a rant, I'm guessing I'm missing some knowledge)

Anyway, that might be the cause of the missing automatic validation on my end?