Coder Social home page Coder Social logo

Comments (20)

NotJustAToy avatar NotJustAToy commented on September 20, 2024 2

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

from falcon-heavy.

NotJustAToy avatar NotJustAToy commented on September 20, 2024 2

In progress. I plan to finish this weekend.

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024 1

@NotJustAToy Ok, the fix you implemented did get the project to work 🎉 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."}
  1. 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
  1. 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)

from falcon-heavy.

NotJustAToy avatar NotJustAToy commented on September 20, 2024

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

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024

@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.

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024

@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

from falcon-heavy.

NotJustAToy avatar NotJustAToy commented on September 20, 2024

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

from falcon-heavy.

NotJustAToy avatar NotJustAToy commented on September 20, 2024

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

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024

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

from falcon-heavy.

NotJustAToy avatar NotJustAToy commented on September 20, 2024

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 oneOf can 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.

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024

@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:

  • A Product with a productType of Item should only be allowed to have these fields:

    1. productStatus (from Product required fields)
    2. productType (from Product required fields)
    3. sku (from Product required fields)
    4. simpleName (from Item required fields)
    5. fullName (from Item required fields)
  • A Product with a productType of Component should only be allowed to have these fields:

    1. productStatus (from Product required fields)
    2. productType (from Product required fields)
    3. sku (from Product required fields)
    4. cartons (from Component required fields)
    5. minimumOrderQty (from Component required fields)
    6. simpleName (from Component required fields)
  • A Product with a productType of Kit should only be allowed to have these fields:

    1. productStatus (from Product required fields)
    2. productType (from Product required fields)
    3. sku (from Product required fields)
    4. kitItems (from Kit required fields)

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
  }]
}]

from falcon-heavy.

NotJustAToy avatar NotJustAToy commented on September 20, 2024
  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.

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024

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.

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024

@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.

from falcon-heavy.

NotJustAToy avatar NotJustAToy commented on September 20, 2024

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

from falcon-heavy.

NotJustAToy avatar NotJustAToy commented on September 20, 2024

@caffeinatedMike can you review these changes?

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024

@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!

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024

@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'

from falcon-heavy.

NotJustAToy avatar NotJustAToy commented on September 20, 2024

@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):

  • properties - contains explicitly defined properties in properties
  • pattern_properties - contains properties defined in x-patternProperties (analog of patternProperties from json-schema)
  • additional_properties - properties that allowed by additionalProperties
    All of these submaps i find useful for developers.

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.

from falcon-heavy.

caffeinatedMike avatar caffeinatedMike commented on September 20, 2024
  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!

from falcon-heavy.

Related Issues (3)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.