CreateAPI / URLQueryEncoder

URL query encoder with support for all OpenAPI serialization options
MIT License
41 stars 7 forks source link

Dictionary of [String: Object] not encoded correctly? #1

Open kerrmarin opened 2 years ago

kerrmarin commented 2 years ago

When encoding a dictionary that contains objects with form style and true explode value, the key for each parameter should be the key for each entry in the dictionary, not the key that is passed in.

This is the behaviour experienced when using https://editor.swagger.io with the following yaml file:

openapi: 3.0.3
info:
  title: Test API
  version: v1
paths:
  /products:
    get:
      summary: Get list of products
      parameters:
        - name: filters
          in: query
          schema:
            $ref: '#/components/schemas/ProductFilterRequestQueryParameter'
      responses:
        '204':
          description: OK
components:
  schemas:
    ProductFilterRequestQueryParameter:
      type: object
      description: |
        A set of constraints placed on filters, keyed by the filter name, and containg an operator and an array of values.
      additionalProperties:
        $ref: '#/components/schemas/ProductFilterRequestConstraint'
      example:
        color:
          operator: eq
          value:
            - blue_code
        material:
          operator: in
          value:
            - gold_code
            - blue_code
    ProductFilterRequestConstraint:
      type: object
      description: |
        A constraint placed on a filter. Constraints can either be "in" (an array) or "equals" (to a value).
      properties:
        operator:
          type: string
        value:
          type: array
          items:
            type: string
      required:
        - operator
        - value

You can see the results of the query produced by "trying out" the request in editor.swagger.io.

However, this package produces a query string that ignores the keys in the dictionary and flattens the values.

A failing test representing the expected output:

    func testEncodeDictionaryWithObject() {

        struct Filter: Codable {
            let `operator`: String
            let value: [String]
        }

        // GIVEN
        let parameter = [
            "color": Filter(operator: "eq", value: ["blue"]),
            "material": Filter(operator: "in", value: ["leather", "metal"])
        ]
        // THEN
        let query = URLQueryEncoder().encode(parameter, forKey: "filters").percentEncodedQuery
        // produces query == "filters=in&filters=leather&filters=metal&filters=eq&filters=blue"
        XCTAssertEqual(query, "color[operator]=eq&color[value]=blue&material[operator]=in&material[value]=leather&material[value]=metal")
    }

I am aware that the output produced by editor.swagger.io seems to be encoding the object as deepObject style while expecting a form style, so perhaps this is not a bug in this package but nonetheless I thought it important to raise an issue to discuss.

As far as I can tell, there is no official OpenAPI Spec support for deeply-nested objects. There is some discussion here with links to other discussions too. Perhaps it would be useful to define what this package will do with deep objects, and officially add support for it?

liamnichols commented 1 year ago

Thanks for raising this @kerrmarin and sorry for the delay in responding! It's a very interesting issue but I don't really have much experience in this area myself and I don't have any good answers unfortunately.

As far as I can tell, there is no official OpenAPI Spec support for deeply-nested objects. There is some discussion https://github.com/swagger-api/swagger-editor/issues/1954 with links to other discussions too. Perhaps it would be useful to define what this package will do with deep objects, and officially add support for it?

Yeah this is a shame. I'd be cautious about changing any behaviour ourselves, although it's interesting that the swagger editor does use the deepObject style. Documenting the behaviour certainly sounds like a good idea though.

If you have a moment, pushing up a Pull Request to add a note to the README that either links to this ticket or to some of the other discussions that you linked to might be a helpful addition 🙇