NotJustAToy / falcon-heavy

The framework for building app backends and microservices by specification-first API design approach based on the OpenAPI Specification 3
Apache License 2.0
23 stars 1 forks source link

Can I get some sample code showing how to use this with a Flask app? #1

Closed caffeinatedMike closed 4 years ago

caffeinatedMike commented 4 years ago

I have built an API with Flask, Flask-Restful, and OpenAPI 3.0 (formerly Swagger). The problem I'm facing is validating incoming requests against the spec.

So far I have tried Flasgger and Connexion, which both work with the Flask framework. However, Flasgger's OAS3 support isn't complete and fails to validate any incoming data. And while Connexion's OAS3 support is broader, it still fails to validate more complex scenerios, such as conditional parameters using the newly-introduced oneOf and allOf properties of the latest OAS spec.

Hence, why I'm looking for a package that can provide more reliable results. That's where I found this repository through a few StackOverflow answers (example) and listed on the OpenAPI.Tools site.

The only problem is I don't see any sort of documentation on where to begin utilizing this tool. I would like to add decorators to my flask-restful Resource classes that directly map to each API verb (ie get, put, post, patch) and correspond to the Flask routes. Can you possibly add some documentation on how to accomplish this?

NotJustAToy commented 4 years ago

I hope this example will be useful for you. Documentation will be soon.

caffeinatedMike commented 4 years ago

@NotJustAToy That basic working example is a great start, thanks! However, I'll need to play around with it to see how sound it is. I'll mess with the example some tonight and report back here. Looking forward to diving into the documentation when it's ready.

caffeinatedMike commented 4 years ago

@NotJustAToy I attempted to test the example you provided last night. However, it looks like falcon-heavy fails to correctly validate even that simple spec. Below is the traceback of the error.

Python 3.7.6 (tags/v3.7.6:43364a7ae0, Dec 19 2019, 00:42:30) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license()" for more information.
>>> 
== RESTART: C:\Users\mhill\Downloads\falcon-heavy-flask-example-master\app.py ==
Traceback (most recent call last):
  File "C:\Users\mhill\Downloads\falcon-heavy-flask-example-master\app.py", line 21, in <module>
    from openapi import openapi, operations
  File "C:\Users\mhill\Downloads\falcon-heavy-flask-example-master\openapi.py", line 73, in <module>
    operations = OpenAPIOperations.from_file(os.path.join(os.path.dirname(__file__), 'petstore.yaml'))
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\contrib\operations.py", line 71, in from_file
    path, handlers=handlers)
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\core\openapi\utils.py", line 46, in load_specification
    **make_specification_conversion_context(base_uri, referrer, handlers=handlers)
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\core\openapi\openapi.py", line 102, in convert
    result: ty.Optional[OpenAPIObject] = super(OpenAPIObjectType, self).convert(value, path, **context)
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\core\types\base.py", line 250, in convert
    result = self._convert(value, path, entity=entity, **context)
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\core\types\object.py", line 397, in _convert
    raise SchemaError(*errors)
falcon_heavy.core.types.exceptions.SchemaError: C:\Users\mhill\Downloads\falcon-heavy-flask-example-master\petstore.yaml#/paths/~1pets/get/responses/200/content/application~1json/schema: Couldn't resolve reference
C:\Users\mhill\Downloads\falcon-heavy-flask-example-master\petstore.yaml#/paths/~1pets/get/responses/default/content/application~1json/schema: Couldn't resolve reference
C:\Users\mhill\Downloads\falcon-heavy-flask-example-master\petstore.yaml#/paths/~1pets/post/responses/default/content/application~1json/schema: Couldn't resolve reference
C:\Users\mhill\Downloads\falcon-heavy-flask-example-master\petstore.yaml#/paths/~1pets~1{petId}/get/responses/200/content/application~1json/schema: Couldn't resolve reference
C:\Users\mhill\Downloads\falcon-heavy-flask-example-master\petstore.yaml#/paths/~1pets~1{petId}/get/responses/default/content/application~1json/schema: Couldn't resolve reference
C:\Users\mhill\Downloads\falcon-heavy-flask-example-master\petstore.yaml#/components/schemas/Pets/items: Couldn't resolve reference
NotJustAToy commented 4 years ago

Yes, I get it. I will try to quickly adapt it to Windows.

NotJustAToy commented 4 years ago

I fixed the problem and updated the dependencies. Please try again.

caffeinatedMike commented 4 years ago

Thanks! Will have a look tomorrow and report back any issues.

caffeinatedMike commented 4 years ago

@NotJustAToy Ok, the fix you implemented did get the project to work :tada: But, there are still a few issues I think prevent this package from being ready for prime-time:

  1. Proper 500 responses missing explanations.
    Example: If I update the POST /pets endpoint like so (preventing additionalProperties)
    post:
    summary: Create a pet
    operationId: createPets
    tags:
    - pets
    requestBody:
    content:
      application/json:
        schema:
          required:
            - name
          properties:
            name:
              type: string
              minLength: 1
          additionalProperties: false

    And send a body such as

    {"name":"teddy", "chip_id": 3}

    It does react properly, throwing a 500 Internal Server Error. I see in the python console the explanation

    falcon_heavy.core.types.exceptions.SchemaError: #/content: When `additionalProperties' is False, no unspecified properties are allowed. The following unspecified properties where found: 'chip_id'

    However, it doesn't explain what was wrong with the request in the response. Like Flasgger and Connexion, it should report the error back to the sender, so they can make sure the payload's sent accordingly. Expected response should be something along the lines of:

    {"message": "'chip_id' is not a valid property."}
  2. explode is not handled properly with query parameters when style is set to form and using oneOf to restrict the parameter combinations to a select number of schemas.
    Example:
    paths:
    /pets:
    delete:
      tags:
      - pricing
      summary: Delete existing Product Pricing
      operationId: deletePets
      parameters:
        - in: query
          name: filter
          style: form
          explode: true
          required: true
          schema:
            oneOf:
              - $ref: '#/components/schemas/SingleSku'
              - $ref: '#/components/schemas/SingleRetailer'
              - $ref: '#/components/schemas/RetailerSku'
              - $ref: '#/components/schemas/RegionSku'
            additionalProperties: false
    components:
    schemas:
    SingleSku:
      title: Single Sku for All Retailers
      description: Deletes the provided SKU from all Retailer price lists. This will remove the product from all Retailer sites.
      type: object
      properties:
        sku:
          type: string
      required:
        - sku
    RetailerSku:
      title: Single Sku for Retailer
      description: Deletes the provided SKU from the provided Retailer's price list. This will only remove the product from that specific Retailer's site.
      type: object
      properties:
        sku:
          type: string
        retailerId:
          type: integer
          format: int64
      required:
        - sku
        - retailerId
    SingleRetailer:
      title: All Pricing for Single Retailer
      description: Deletes the provided Retailer's entire price list. This will remove all products from that specific Retailer's site.
      type: object
      properties:
        retailerId:
          type: integer
          format: int64
      required:
        - retailerId
    RegionSku:
      title: Single Sku for Region
      description: Deletes the provided SKU from the provided Regional price list. This will remove the product from all  Retailers' sites that are assigned to that Regional price list.
      type: object
      properties:
        sku:
          type: string
        regionId:
          type: integer
          format: int64
      required:
        - sku
        - regionId

    Python Function

    @openapi
    def deletePets(request):
    filter = request.query_params.get('filter')
    print(filter)
    return OpenAPIJSONResponse(filter)

    When sending the request DELETE http://127.0.0.1:5000/pets?sku=abc123 a 400 response is recieved and the python console states

    falcon_heavy.core.types.exceptions.SchemaError: #/parameters/query/filter: When 'additionalProperties' is False, no unspecified properties are allowed. The following unspecified properties were found: 'sku'

    Even when commenting out additionalProperties: false a 400 response is received and the python console states

    falcon_heavy.core.types.exceptions.SchemaError: #/parameters/query/filter: Missing required parameter
  3. Discriminator is not acting as expected when additional properties are provided. When specifying a discriminator and setting additionalProperties: false on one of the discriminatory components, only the required fields of the discriminatory component are accepted when those required fields should be combined with the required fields of the component that has the discrimantor property. Given the following yaml
    paths:
    /pets:
    post:
      summary: dkdkdk
      operationId: createPet
      tags:
        - pets
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Product'
      responses:
        '200':
          description: Expected response to a valid request
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Product"
        '404':
          description: Pet not found
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
      responses:
        '200':
          description: A paged array of pets
          headers:
            x-next:
              description: A link to the next page of responses
              schema:
                type: string
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: "#/components/schemas/Product"
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"
    components:
    schemas:
    KitItem:
      type: object
      properties:
        sku:
          type: string
        qty:
          type: integer
          format: int32
          minimum: 1
          default: 1
      required:
        - sku
        - qty
    Item:
      description: A representation of an Item
      allOf:
        - $ref: '#/components/schemas/Product'
        - type: object
          additionalProperties: false
          properties:
            productLength:
              type: number
              description: Length of the Product (in inches)
              format: float
              example: 36.0
            productWidth:
              type: number
              description: Width of the Product (in inches)
              format: float
              example: 59.25
            productHeight:
              type: number
              description: Height of the Product (in inches)
              format: float
              example: 36.0
            productVolume:
              type: number
              description: Volume of the product (in Cubic Feet)
              format: float
              example: 121.0
            productWeight:
              type: number
              description: Weight of the product (in pounds)
              format: float
              example: 121.0
            packageLength:
              type: number
              description: Length of the packaging in which the Product ships (in inches)
              format: float
              example: 36.0
            packageWidth:
              type: number
              description: Width of the packaging in which the Product ships (in inches)
              format: float
              example: 59.25
            packageHeight:
              type: number
              description: Height of the packaging in which the Product ships (in inches)
              format: float
              example: 36.0
            cartons:
              type: number
              description: Number of cartons the product ships in
              format: int32
              minimum: 1
              default: 1
            minimumOrderQty:
              type: number
              description: |
                If the price provided is the price of the entire Product, enter 1.  
                An example of when you would use a number greater than 1 is:  
                If there are two chairs to a carton, but the Price provided is for just one chair, then enter 2.
              format: int32
              minimum: 1
              default: 1
            simpleName:
              type: string
            fullName:
              type: string
            featureImage:
              type: string
            category:
              type: integer
              format: int64
          required:
            - cartons
            - minimumOrderQty
            - simpleName
            - fullName
            - featureImage
            - category
    Component:
      description: A product that can only be sold as part of a bigger item, such as a mirror that is only sold with the accompanying dresser.
      allOf:
        - $ref: '#/components/schemas/Product'
        - type: object
          additionalProperties: false
          properties:
            cartons:
              type: number
              description: Number of cartons the product ships in
              format: int32
              minimum: 1
              default: 1
            minimumOrderQty:
              type: number
              description: |
                If the price provided is the price of the entire Product, enter 1.  
                An example of when you would use a number greater than 1 is:  
                If there are two chairs to a carton, but the Price provided is for just one chair, then enter 2.
              format: int32
              minimum: 1
              default: 1
            simpleName:
              type: string
          required:
            - cartons
            - minimumOrderQty
            - simpleName
    Kit:
      description: A list of `KitItem` objects
      allOf:
        - $ref: '#/components/schemas/Product'
        - type: object
          additionalProperties: false
          properties:
            kitItems:
              type: array
              items:
                $ref: '#/components/schemas/KitItem'
          required:
            - kitItems
            - fullName
            - featureImage
            - category
    Product:
      type: object
      required:
        - productStatus
        - productType
        - sku
      discriminator:
        propertyName: productType
        mapping:
          Item: '#/components/schemas/Item'
          Component: '#/components/schemas/Component'
          Kit: '#/components/schemas/Kit'
      properties:
        id:
          description: Product ID
          type: integer
          format: int64
        sku:
          description: Product SKU
          type: string
          example: "1006-10"
        upc:
          description: Universal Product Code
          type: string
          example: "012345678905"
        productType:
          description: |
            If the Product is able to be sold by itself, then enter Item.  
            If the Product is part of a larger product and cannot be sold by itself,
            then enter Component.  
            If the Product is comprised of multiple items and you've provided the items, then enter Kit.
          type: string
        productStatus:
          type: string
          description: Product status
          enum:
          - Active
          - Discontinued
    requestBodies:
    Product:
      content:
        application/json:
          schema:
            allOf:
              - $ref: '#/components/schemas/Product'
      description: The Product object you would like to add
      required: true

    With the following Python function

    @openapi
    def createPet(request):
    body = request.content
    return OpenAPIJSONResponse(body)

    And Sending the following payload

    {
    "kitItems": [],
    "productType": "Kit",
    "productStatus": "Active",
    "sku": "abc123"
    }

    Results in a 400 response and the following to be printed in the console

    falcon_heavy.core.types.exceptions.SchemaError: #/content: Does not match all schemas from 'allOf'. Invalid schema indexes: 1
    #/content/1: When 'additionalProperties' is False, no unspecified properties are allowed. The following unspecified properties were found: 'productStatus', 'productType', 'sku'

    The expected behavior would be for both the main component's and its discriminator's component's required fields to be combined. Thus, the following should all be required in this case:

    • productStatus (From Product required array)
    • productType (From Product required array)
    • sku (From Product required array)
    • kitItems (From Kit required array)
    • fullName (From Kit required array)
    • featureImage (From Kit required array)
    • category (From Kit required array)
NotJustAToy commented 4 years ago

Thanks for the feedback.

  1. You should generate response with error yourself by overriding _handle_invalid_request in decorator. https://github.com/NotJustAToy/falcon-heavy-flask-example/blob/master/openapi.py#L60 It need because all responses must be valid through specification. In one of my project i used the following code:

    def _handle_invalid_request(self, request, operation, instance, exception):
        logger.error("Errors at request at '%s' has occurred:\n%s", request.path, str(exception))
    
        errors = []
        for error in exception.errors:
            place, *others = error.path.parts
    
            if place == 'content':
                source = {
                    'pointer': '/'.join(others)
                }
    
            elif place == 'parameters':
                location, name, *others = others
                parameter = {
                    'in': location,
                    'name': name
                }
    
                if others:
                    parameter['pointer'] = '/'.join(others)
    
                source = {
                    'parameter': parameter
                }
    
            else:
                raise ValueError("Unexpected exception format")
    
            errors.append({
                'message': error.message,
                'source': source
            })
    
        return OpenAPIJSONResponse({'errors': errors}, status_code=400)

and the following schema

    BadRequest:
      description: Bad request
      content:
        application/json:
          schema:
            properties:
              errors:
                type: array
                items:
                  properties:
                    message:
                      type: string
                    source:
                      anyOf:
                        - properties:
                            pointer:
                              type: string
                          required:
                            - pointer
                          additionalProperties: false
                        - properties:
                            parameter:
                              properties:
                                in:
                                  type: string
                                  enum:
                                    - path
                                    - query
                                    - header
                                    - cookie
                                name:
                                  type: string
                                pointer:
                                  type: string
                              required:
                                - in
                                - name
                              additionalProperties: false
                          required:
                            - parameter
                          additionalProperties: false
                  required:
                    - message
                    - source
            required:
              - errors
            additionalProperties: false
  1. OAS schema property is not similar that jsonschema. Falcon-Heavy use schema type inferring by keywords (if you don't specify it). In this example Falcon-Heavy think that you provide a schema with type object (additionalProperties is keyword of object). In second case it, i think, limitation of OAS 3. I not found right way for validate such parameters. In second case Falcon-Heavy can't infer type for oneOf and not generate right validator (subschemas of oneOfcan be schemas with any possible types: one of integer or string, object or integer and so on). I found big limitation on polymorphic parameters in OAS 3. I think better not used polymorphic types in parameters or used another form providing query parameters (maybe with explode==false)
  2. By OAS 3 payload must be valid through all subschemas separately. In this case Kit restrict possible properties by additionalProperties==false in second subschema. Can you provide some official examples where explain how it should work? I found only bit group of examples that not fully explain it.
caffeinatedMike commented 4 years ago

@NotJustAToy

  1. Makes sense, thank you for the example. I can definitely make use of this as a guideline.
  2. Other than it being supported in the OAS3 docs and is, in fact, the default serialization method for query parameters:

    The default serialization method is style: form and explode: true.

    The reasoning for my using the style: form/explode: true combination is how it is rendered/displayed in the documentation. Here's the way it appears in the ReDoc UI, which is very reader-friendly and ensures no confusion on the acceptable combinations. This output is accomplished with the following working example that give you the aforementioned valid visual output.

openapi: 3.0.1
servers:
  - url: //127.0.0.1:5000
    description: Development Server
  version: 1.0.0
  title: Swagger Petstore
  termsOfService: 'http://swagger.io/terms/'
  contact:
    url: https://github.com/Redocly/redoc
  x-logo:
    url: 'https://redocly.github.io/redoc/petstore-logo.png'
    altText: Petstore logo
  license:
    name: Apache 2.0
    url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
externalDocs:
  description: Find out how to create Github repo for your OpenAPI spec.
  url: 'https://github.com/Rebilly/generator-openapi-repo'
paths:
  /pricing:
    delete:
      tags:
      - pricing
      summary: Delete existing Product Pricing
      operationId: deletePricing
      parameters:
        - in: query
          name: filter
          style: form
          explode: true
          required: true
          schema:
            oneOf:
              - $ref: '#/components/schemas/SingleSku'
              - $ref: '#/components/schemas/SingleRetailer'
              - $ref: '#/components/schemas/RetailerSku'
              - $ref: '#/components/schemas/RegionSku'
            additionalProperties: false
      responses:
        204:
          description: Product pricing deleted
components:
  schemas:
    SingleSku:
      title: Single Sku for All Retailers
      description: Deletes the provided SKU from all Retailer price lists. This will remove the product from all Retailer sites.
      type: object
      properties:
        sku:
          type: string
      required:
        - sku
    RetailerSku:
      title: Single Sku for Single Retailer
      description: Deletes the provided SKU from the provided Retailer's price list. This will only remove the product from that specific Retailer's site.
      type: object
      properties:
        sku:
          type: string
        retailerId:
          type: integer
          format: int64
      required:
        - sku
        - retailerId
    SingleRetailer:
      title: All Pricing for Single Retailer
      description: Deletes the provided Retailer's entire price list. This will remove all products from that specific Retailer's site.
      type: object
      properties:
        retailerId:
          type: integer
          format: int64
      required:
        - retailerId
    RegionSku:
      title: Single Sku for Region
      description: Deletes the provided SKU from the provided Regional price list. This will remove the product from all  Retailers' sites that are assigned to that Regional price list.
      type: object
      properties:
        sku:
          type: string
        regionId:
          type: integer
          format: int64
      required:
        - sku
        - regionId
  1. Here is a working example that gives you the (incorrect) valid responses due to the lack of additionalProperties: true.
openapi: "3.0.0"
info:
  version: 1.0.0
  title: Swagger Petstore
  license:
    name: MIT
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets:
    post:
      summary: Demonstrates discriminatory issue. If `additionalProperties` is _not_ set to `false`, then you can pass additional fields and still get a (incorrectly) valid response.
      operationId: createPet
      tags:
        - pets
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Product'
            examples:
              item:
                summary: Should not allow `kitItems`
                value:
                  productStatus: Active
                  productType: Item
                  sku: abc123
                  simpleName: Test
                  fullName: Full Test
                  kitItems:
                  - sku: item1
                    qty: 1
                  - sku: item2
                    qty: 2
              component:
                summary: Should not allow `fullName`
                value:
                  productStatus: Active
                  productType: Component
                  sku: abc123
                  cartons: 1
                  minimumOrderQty: 1
                  simpleName: Test
                  fullName: Component Test
              kit:
                summary: Should not allow `simpleName`
                value:
                  productStatus: Active
                  productType: Item
                  sku: abc123
                  kitItems:
                  - sku: item1
                    qty: 1
                  - sku: item2
                    qty: 2
                  simpleName: Test
      responses:
        '200':
          description: Just returning the payload to confirm intended schema was sent
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Product"
components:
  schemas:
    KitItem:
      type: object
      properties:
        sku:
          type: string
        qty:
          type: integer
          format: int32
          minimum: 1
          default: 1
      required:
        - sku
        - qty
    Item:
      description: A representation of an Item
      allOf:
        - $ref: '#/components/schemas/Product'
        - type: object
          properties:
            simpleName:
              type: string
            fullName:
              type: string
          required:
            - simpleName
            - fullName
    Component:
      description: A product that can only be sold as part of a bigger item
      allOf:
        - $ref: '#/components/schemas/Product'
        - type: object
          properties:
            cartons:
              type: number
              description: Number of cartons the product ships in
              format: int32
              minimum: 1
              default: 1
            minimumOrderQty:
              type: number
              format: int32
              minimum: 1
              default: 1
            simpleName:
              type: string
          required:
            - cartons
            - minimumOrderQty
            - simpleName
    Kit:
      description: A product comprised of other Items and/or Components
      allOf:
        - $ref: '#/components/schemas/Product'
        - type: object
          properties:
            kitItems:
              type: array
              items:
                $ref: '#/components/schemas/KitItem'
          required:
            - kitItems
    Product:
      type: object
      required:
        - productStatus
        - productType
        - sku
      discriminator:
        propertyName: productType
        mapping:
          Item: '#/components/schemas/Item'
          Component: '#/components/schemas/Component'
          Kit: '#/components/schemas/Kit'
      properties:
        sku:
          description: Product SKU
          type: string
        productType:
          type: string
        productStatus:
          type: string
          description: Product status
          enum:
          - Active
          - Discontinued

Given the above yaml, here are the expected restraints:

And the json payloads to demonstrate how this doesn't work

[{
  "productStatus": "Active",
  "productType": "Item",
  "sku": "abc123",
  "simpleName": "Test",
  "fullName": "Full Test",
  "kitItems": [{
    "sku": "we-shouldnt-be-allowed-1",
    "qty": 1
  },{
    "sku": "we-shouldnt-be-allowed-2",
    "qty": 2
  }]
},{
  "productStatus": "Active",
  "productType": "Component",
  "sku": "abc123",
  "simpleName": "Test",
  "cartons": 1,
  "minimumOrderQty": 1,
  "fullName": "I shouldn't be allowed"
},{
  "productStatus": "Active",
  "productType": "Kit",
  "sku": "abc123",
  "simpleName": "I'm Not Allowed",
  "kitItems": [{
    "sku": "component-1",
    "qty": 1
  },{
    "sku": "component-2",
    "qty": 2
  }]
}]
NotJustAToy commented 4 years ago
  1. Glad it helped.
  2. I understand you, but parameter styles require type (primitive, array or object) before they can be applied. We can't infer type of oneOf in general case. But I can make this mechanism sensitive to the vendor parameter (like x-type), to explicitly indicate the type in such cases or infer type by first subschema in oneOf.
  3. Yeah, it's big limitation. The following confuse me:

    
    components:
    schemas:
    Dog:
      allOf: 
        - $ref: '#/components/schemas/Animal'
        - additionalProperties: false
        - additionalProperties: true
    
    Animal:
      type: object
      discriminator:
        propertyName: type
        mapping:
          Dog: '#/components/schemas/Dog'
      properties:
        type:
          type: string
        name:
          type: string
      required:
        - name

In this example `Dog` schema has two conflicting subschemas with different `additionalProperties`. I don't know how it should work in general case.
caffeinatedMike commented 4 years ago

Re: 2

But I can make this mechanism sensitive to the vendor parameter (like x-type), to explicitly indicate the type in such cases or infer type by first subschema in oneOf.

I think either of these options would suffice (at least for my use-cases) because I believe in most use-cases (not just for mine) all schemas provided would be of the same type.

I think symantically, that's how REST APIs are intended to function. I don't think it's normal for an endpoint to accept a dictionary OR a list. IMO if the developer is trying to accept both it's poor design and is indicative of there needing to be two separate endpoints (ie. /product should accept a dictionary/object, while /products would accept a list/array (of dictionary/objects)).

Re: 3

To me, your example isn't valid. The only reason I can think that it would be marked valid by swagger is due to the nature in which the yaml files are processed (from top-down). So, given your example, I would expect Dog would accept additionalProperties because additionalProperties: true overwrites the additionalProperties: false from a top-down point-of-view.

Interestingly, I did find the following snippet explaining the shortcoming of combining schemas and utilizing additionalProperties: false in the json-schema.org documentation

This works, but what if we wanted to restrict the schema so no additional properties are allowed? One might try adding the additionalProperties: false line below:

{
  "definitions": {
    "address": {
      "type": "object",
      "properties": {
        "street_address": { "type": "string" },
        "city":           { "type": "string" },
        "state":          { "type": "string" }
      },
      "required": ["street_address", "city", "state"]
    }
  },

  "allOf": [
    { "$ref": "#/definitions/address" },
    { "properties": {
        "type": { "enum": [ "residential", "business" ] }
      }
    }
  ],

  "additionalProperties": false
}

Then use the following payload

{
   "street_address": "1600 Pennsylvania Avenue NW",
   "city": "Washington",
   "state": "DC",
   "type": "business"
}

Unfortunately, now the schema will reject everything. This is because the Properties refers to the entire schema. And that entire schema includes no properties, and knows nothing about the properties in the subschemas inside of the allOf array.

This shortcoming is perhaps one of the biggest surprises of the combining operations in JSON schema: it does not behave like inheritance in an object-oriented language. There are some proposals to address this in the next version of the JSON schema specification.

When it comes to this scenario I think the logical behavior would be to inherit the additional schema's properties, then honor an additionalProperties: false declaration on the sub-schema that inherited the parent schema. I'll use your example to demonstrate:

components:
  schemas:
    Dog:
      allOf: 
        - $ref: '#/components/schemas/Animal'
        - type: object
          additionalProperties: false
          required:
          - breed
          properties:
            breed:
              type: string
              enum:
              - Poodle
              - Siberian Husky
              - Chihuahua
              - Yorkie
    Animal:
      type: object
      discriminator:
        propertyName: type
        mapping:
          Dog: '#/components/schemas/Dog'
      properties:
        type:
          type: string
        name:
          type: string
      required:
        - name
        - type

Given the above, I think it's logical for Dog to require three properties:

  1. type (from Pet schema)
  2. name (from Pet schema)
  3. breed (from Dog schema)

I think it's also logical for Dog to prevent the allowance of any additionalProperties, given the presence of the additionalProperties: false on that particular subschema.

However, also given the above, I think it's logical for Pet to allow additionalProperties (given it is not of type: Dog).

Does that all make sense?

In Summary:

  1. RE: 2 Can we make either of those mechanisms happen? I could really use the ability to have explodable parameters.
  2. RE: 3 Would it be plausible for this to be implemented in this project? I know that OAS3 utilizes json-schema under the hood and that this is a limitation with json-schema, but would it be possible to implement a workaround that deals with this situation properly in this downstream project? The upstream projects` (json-schema and oas3) processes seem to drag on sometimes spanning years and multiple major releases before anything is changed.
NotJustAToy commented 4 years ago

It makes sense, I will do it. Thank you.

caffeinatedMike commented 4 years ago

@NotJustAToy Has any progress been made on this? I'm happy to help however I can to facilitate improvements, be it by PR (if told where edits need to be made) or by continually testing and providing feedback.

NotJustAToy commented 4 years ago

In progress. I plan to finish this weekend.

NotJustAToy commented 4 years ago

Done. Added http method to OpenAPIRequest, improved parameter type inferring by polymorphic schemas, added ability of set parameter type explicitly (through x-parameterType property), added ability of merging subschemas of allOf (by x-merge property). x-parameterType example (implicitly):

paths:
  /pets:
    delete:
      tags:
      - pricing
      summary: Delete existing Product Pricing
      operationId: deletePets
      parameters:
        - in: query
          name: filter
          style: form
          explode: true
          required: true
          schema:
            oneOf:
              - $ref: '#/components/schemas/SingleSku'
              - $ref: '#/components/schemas/SingleRetailer'
              - $ref: '#/components/schemas/RetailerSku'
              - $ref: '#/components/schemas/RegionSku'

or (explicitly)

paths:
  /pets:
    delete:
      tags:
      - pricing
      summary: Delete existing Product Pricing
      operationId: deletePets
      parameters:
        - in: query
          name: filter
          style: form
          explode: true
          required: true
          x-parameterType: object
          schema:
            oneOf:
              - $ref: '#/components/schemas/SingleSku'
              - $ref: '#/components/schemas/SingleRetailer'
              - $ref: '#/components/schemas/RetailerSku'
              - $ref: '#/components/schemas/RegionSku'

x-merge example:

components:
  schemas:
    Dog:
      x-merge: true
      allOf: 
        - $ref: '#/components/schemas/Animal'
        - type: object
          additionalProperties: false
          required:
          - breed
          properties:
            breed:
              type: string
              enum:
              - Poodle
              - Siberian Husky
              - Chihuahua
              - Yorkie
    Animal:
      type: object
      discriminator:
        propertyName: type
        mapping:
          Dog: '#/components/schemas/Dog'
      properties:
        type:
          type: string
        name:
          type: string
      required:
        - name
        - type

falcon-heavy==0.1.2 is released

NotJustAToy commented 4 years ago

@caffeinatedMike can you review these changes?

caffeinatedMike commented 4 years ago

@NotJustAToy Absolutely, I'm sorry for the delay. Work's been hectic. I will review these changes once I get home from work and report back tonight. Thank you again for all the effort!

caffeinatedMike commented 4 years ago

@NotJustAToy Here's my input after having a chance to fiddle with the update

x-parameter-type

Working like a charm! Just curious though. I noticed when using the oneOf property, the filter parameter has 3 dictionaries inside it (I'm assuming one for each of the oneOf schemas). Was this intended or no? It's no big deal as long as last 3 commented-out print statements suffice to grab the needed data in the function.
Example:

@openapi
def deletePricing(request):
    print(request.query_params)
    #print(request.query_params.get('filter', {}).get('regionId'))
    #print(request.query_params.get('filter', {}).get('retailerId'))
    #print(request.query_params.get('filter', {}).get('sku'))
    return OpenAPIResponse(status_code=204)

#DELETE 127.0.0.1:5000/pricing?sku=abc123&regionId=456
#Output: `{'filter': Object({'regionId': 456, 'sku': 'abc123'}, {}, {})}`

I can also confirm that invalid combinations not matching oneOf the schemas are now thrown (y)

x-merge and additionalProperties: false

Also worked like a charm! But, with one caveat. I couldn't use the parent schema that had the discriminator, I had to utilize oneOf with the child schemas that inherited the parent schema. Was this intended? Example:

      responses:
        '200':
          description: Just returning the payload to confirm intended schema was sent
          content:
            application/json:
              schema:
                #$ref: "#/components/schemas/Product"
                oneOf:
                - $ref: '#/components/schemas/Kit'
                - $ref: '#/components/schemas/Item'
                - $ref: '#/components/schemas/Component'

Side note, referring back to your BadRequest schema

paths:
  /products:
    post:
      summary: Demonstrates discriminatory issue. If `additionalProperties` is _not_ set to `false`, then you can pass additional fields and still get a (incorrectly) valid response.
      operationId: createProduct
      tags:
        - pets
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Product'
      responses:
        '200':
          #.......
        '400':
          $ref: '#/components/schemas/BadRequest'
        # The following works though
        # '400':
          # description: Bad request
          # content:
            # application/json:
              # schema:
                # type: object

When I attempted to add that to my spec file I received the following error.

= RESTART: C:\Users\mhill\Desktop\falcon-heavy-flask-example-master\app\app.py =
Traceback (most recent call last):
  File "C:\Users\mhill\Desktop\falcon-heavy-flask-example-master\app\app.py", line 21, in <module>
    from openapi import openapi, operations
  File "C:\Users\mhill\Desktop\falcon-heavy-flask-example-master\app\openapi.py", line 105, in <module>
    operations = OpenAPIOperations.from_file(os.path.join(os.path.dirname(__file__), 'petstore.yaml'))
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\contrib\operations.py", line 71, in from_file
    path, handlers=handlers)
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\core\openapi\openapi.py", line 170, in load_specification
    **make_specification_conversion_context(base_uri, referrer, handlers=handlers)
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\core\openapi\openapi.py", line 111, in convert
    result: ty.Optional[OpenAPIObject] = super(OpenAPIObjectType, self).convert(value, path, **context)
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\core\types\base.py", line 250, in convert
    result = self._convert(value, path, entity=entity, **context)
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\core\types\object.py", line 388, in _convert
    value.get(property_name, Undefined), path / property_name, entity=entity, **context)
  File "C:\Users\mhill\AppData\Local\Programs\Python\Python37\lib\site-packages\falcon_heavy\core\openapi\components.py", line 142, in convert
    all_of = schema.all_of
AttributeError: 'ResponseObject' object has no attribute 'all_of'
NotJustAToy commented 4 years ago

@caffeinatedMike Thanks for your review!

I noticed when using the oneOf property, the filter parameter has 3 dictionaries inside it (I'm assuming one for each of the oneOf schemas).

For schemas with type object Falcon-Heavy returns object of class Object. Object is a ChainMap that contains the three following submaps (priority for __getitem__ access is top-down):

But, with one caveat. I couldn't use the parent schema that had the discriminator, I had to utilize oneOf with the child schemas that inherited the parent schema.

Can your expose the full specification? I use the following and it works:

paths:
  /animal:
    get:
      ...
      responses:
        '200':
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Animal"

components:
  schemas:
    Cat:
      x-merge: true
      allOf:
        - $ref: "#/components/schemas/Animal"
        - properties:
            name:
              type: string
          additionalProperties: false
    Animal:
      discriminator:
        propertyName: type
      properties:
        type:
          type: string
      required:
        - type

Side note, referring back to your BadRequest schema

It is happened because component for BadRequest must be placed in components/responses section instead components/schemas. Falcon-Heavy uses cache for all resolvable parts of specification. In this example Falcon-Heavy first resolve BadRequest as ResponseObject and second try test as SchemaObject (mechanism that detect model-level polymorphic schemas). I will think how i can solve that.

caffeinatedMike commented 4 years ago
  1. Ahh, that makes sense. Thanks for the explanation.
  2. My mistake, I think I must've been a bit too mentally exhausted last night while testing. Going through and re-testing it is working as expected.
  3. Whoops, you're right. I forgot to put it in the responses section instead of the schemas section. That fixed it.

I think this means that this issue can be closed now. Thank you so much for you patience and hard work on the project. This is by-far the best and most extensive OAS3 python library to-date!