Skip to content

Empty Dictionary fails to encode #525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
grahamburgsma opened this issue Feb 9, 2024 · 9 comments · Fixed by apple/swift-openapi-runtime#103
Closed

Empty Dictionary fails to encode #525

grahamburgsma opened this issue Feb 9, 2024 · 9 comments · Fixed by apple/swift-openapi-runtime#103
Assignees
Labels
kind/bug Feature doesn't work as expected.
Milestone

Comments

@grahamburgsma
Copy link

grahamburgsma commented Feb 9, 2024

Description

Returning an empty dictionary as the response body fails to encode.

Error thrown:
EncodingError: invalidValue jsonPayload(additionalProperties: [:]) - at : Top-level jsonPayload did not encode any values.

Reproduction

openapi: '3.1.0'

paths:
  /test:
    get:
      operationId: testEmpty
      responses:
        '200':
          description: Successful response
          content:
            application/json:
              schema:
                type: object
                additionalProperties:
                  type: array
                  items:
                    type: string
func testEmpty(_ input: Operations.testEmpty.Input) async throws -> Operations.testEmpty.Output {
    return .ok(.init(body: .json(.init(additionalProperties: [:]))))
}

Package version(s)

OpenAPIKit 3.1.2
swift-openapi-generator 1.2.0
swift-openapi-runtime 1.3.2
swift-openapi-vapor 1.0.0
swift-openapi-context 1.0.0

Expected behavior

The response should be encoded as:

{}

Environment

swift-driver version: 1.87.3 Apple Swift version 5.9.2 (swiftlang-5.9.2.2.56 clang-1500.1.0.2.5)
Target: arm64-apple-macosx14.0
@grahamburgsma grahamburgsma added kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue. labels Feb 9, 2024
@czechboy0
Copy link
Contributor

Seems to be coming from the underlying Foundation.JSONEncoder, so might need to be filed on Foundation unless we identify that the generated code is doing something wrong.

@grahamburgsma
Copy link
Author

@czechboy0 I don't believe so, the following works fine:

let empty = [String: [String]]()
let data = try! JSONEncoder().encode(empty)
print(String(decoding: data, as: UTF8.self)) // prints {}

Could be related to something going on here?

@grahamburgsma
Copy link
Author

Actually I wonder if I've done something wrong in my spec definition as instead of transforming into a dictionary ([String: [String]]) it generates a struct with an additionalProperties property on it which can't be right.

What is the correct way to have a Swift Dictionary type generated?

@czechboy0
Copy link
Contributor

A freeform JSON-based object? That'd be just:

type: object

If you'd like a dictionary of a specific type, for example integers, then:

type: object
additionalProperties:
  type: integer

@grahamburgsma
Copy link
Author

@czechboy0 That doesn't generate a dictionary property though, it gets put inside a wrapper struct which would make working with those types very clumsy. I must be doing something wrong, but this is what I'm seeing.

For example this:

Test:
  type: object
  properties:
    dict:
      type: object
      additionalProperties:
        type: integer

Gets generated into this (simplified):

struct Test {
    struct dictPayload {
        var additionalProperties: [String: Swift.Int]
    }

    var dict: Components.Schemas.Test.dictPayload?
}

Rather than what I would expect (and what all other OpenAPI generators seem to do):

struct Test {
    var dict: [String: Swift.Int]?
}

@czechboy0
Copy link
Contributor

This is expected behavior. If you defined a property in addition to the additionalProperties in the schema for dict, we need a type to define it on. This is to ensure that adding a property doesn't fundamentally change the generated type (from a dictionary to a struct), so it's always a nested type here.

@czechboy0
Copy link
Contributor

@grahamburgsma Does this address your issue or do you believe that there is something we need to change in the generator?

@grahamburgsma
Copy link
Author

@czechboy0 I understand why it generates the code as it does now, thanks for explaining that.

The original issue as described still exists though. An empty dictionary response fails to encode.

@czechboy0
Copy link
Contributor

Gotcha, here's a fix, @grahamburgsma: apple/swift-openapi-runtime#103

@czechboy0 czechboy0 removed the status/triage Collecting information required to triage the issue. label Apr 2, 2024
@czechboy0 czechboy0 self-assigned this Apr 2, 2024
@czechboy0 czechboy0 added this to the Post-1.0 milestone Apr 2, 2024
czechboy0 added a commit to apple/swift-openapi-runtime that referenced this issue Apr 3, 2024
### Motivation

Fixes apple/swift-openapi-generator#525.

Turns out that the mere act of creating a decoding container is
meaningful and we skipped it as an optimization, causing JSONDecoder to
fail for empty dictionaries when used in additional properties.

### Modifications

Remove the extra guards that skipped creating a container, even when we
already know there are no elements.

### Result

No more failures when encoding empty dictionaries in
additionalProperties.

### Test Plan

Tested manually as this requirement seems to be coming out of
JSONDecoder.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Feature doesn't work as expected.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants