Skip to content
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

Support oneOf required sections #739

Open
philippzagar opened this issue Feb 19, 2025 · 5 comments
Open

Support oneOf required sections #739

philippzagar opened this issue Feb 19, 2025 · 5 comments
Labels
area/openapi Adding/updating a feature defined in OpenAPI. kind/enhancement Improvements to existing feature. status/needs-design Needs further discussion and a concrete proposal.

Comments

@philippzagar
Copy link

philippzagar commented Feb 19, 2025

Description

As described in openai/openai-openapi#391, the OpenAI OpenAPI specification contains the following oneOf required definitions:

oneOf:
  - required:
      - vector_store_ids
  - required:
      - vector_stores

While not wrong in OpenAPI 3.0 or 3.1, the swift-openapi-generator doesn't properly recognize that required definition and produces the following warnings:

A property name only appears in the required list, but not in the properties map - this is likely a typo; skipping this property. [context: foundIn=Components.Schemas.CreateAssistantRequest.tool_resourcesPayload.file_searchPayload.Case1Payload (#/components/schemas/CreateAssistantRequest/tool_resources/file_search/case1)/vector_store_ids]

A property name only appears in the required list, but not in the properties map - this is likely a typo; skipping this property. [context: foundIn=Components.Schemas.CreateAssistantRequest.tool_resourcesPayload.file_searchPayload.Case2Payload (#/components/schemas/CreateAssistantRequest/tool_resources/file_search/case2)/vector_stores]

The swift-openapi-generator should support this valid syntax.

Reproduction

The usage of the following OpenAI OpenAPI spec results in the swift-openapi-generator not properly recognizing the required definition and produces warnings.

A small excerpt of the relevant piece of the OpenAI OpenAPI spec:

openapi: 3.0.0
# ...
    tool_resources:
          type: object
          description: >
            A set of resources that are used by the assistant's tools. The
            resources are specific to the type of tool. For example, the
            `code_interpreter` tool requires a list of file IDs, while the
            `file_search` tool requires a list of vector store IDs.
          properties:
            code_interpreter:
              type: object
              properties:
                file_ids:
                  type: array
                  description: >
                    A list of [file](/docs/api-reference/files) IDs made
                    available to the `code_interpreter` tool. There can be a
                    maximum of 20 files associated with the tool.
                  default: []
                  maxItems: 20
                  items:
                    type: string
            file_search:
              type: object
              properties:
                vector_store_ids:
                  type: array
                  description: >
                    The [vector store](/docs/api-reference/vector-stores/object)
                    attached to this assistant. There can be a maximum of 1
                    vector store attached to the assistant.
                  maxItems: 1
                  items:
                    type: string
                vector_stores:
                  type: array
                  description: >
                    A helper to create a [vector
                    store](/docs/api-reference/vector-stores/object) with
                    file_ids and attach it to this assistant. There can be a
                    maximum of 1 vector store attached to the assistant.
                  maxItems: 1
                  items:
                    type: object
                    properties:
                      file_ids:
                        type: array
                        description: >
                          A list of [file](/docs/api-reference/files) IDs to add
                          to the vector store. There can be a maximum of 10000
                          files in a vector store.
                        maxItems: 10000
                        items:
                          type: string
                      chunking_strategy:
                        type: object
                        description: The chunking strategy used to chunk the file(s). If not set, will
                          use the `auto` strategy.
                        oneOf:
                          - type: object
                            title: Auto Chunking Strategy
                            description: The default strategy. This strategy currently uses a
                              `max_chunk_size_tokens` of `800` and
                              `chunk_overlap_tokens` of `400`.
                            additionalProperties: false
                            properties:
                              type:
                                type: string
                                description: Always `auto`.
                                enum:
                                  - auto
                                x-stainless-const: true
                            required:
                              - type
                          - type: object
                            title: Static Chunking Strategy
                            additionalProperties: false
                            properties:
                              type:
                                type: string
                                description: Always `static`.
                                enum:
                                  - static
                                x-stainless-const: true
                              static:
                                type: object
                                additionalProperties: false
                                properties:
                                  max_chunk_size_tokens:
                                    type: integer
                                    minimum: 100
                                    maximum: 4096
                                    description: The maximum number of tokens in each chunk. The default value is
                                      `800`. The minimum value is `100` and the
                                      maximum value is `4096`.
                                  chunk_overlap_tokens:
                                    type: integer
                                    description: >
                                      The number of tokens that overlap between
                                      chunks. The default value is `400`.


                                      Note that the overlap must not exceed half
                                      of `max_chunk_size_tokens`.
                                required:
                                  - max_chunk_size_tokens
                                  - chunk_overlap_tokens
                            required:
                              - type
                              - static
                        x-oaiExpandable: true
                      metadata:
                        $ref: "#/components/schemas/Metadata"
              oneOf:     # Syntax not recognized by the swift-openapi-generator
                - required:
                    - vector_store_ids
                - required:
                    - vector_stores
          nullable: true
# ...

Swift Generator config:

generate:
  - types
  - client
accessModifier: package

Results in the following warnings:

A property name only appears in the required list, but not in the properties map - this is likely a typo; skipping this property. [context: foundIn=Components.Schemas.CreateAssistantRequest.tool_resourcesPayload.file_searchPayload.Case1Payload (#/components/schemas/CreateAssistantRequest/tool_resources/file_search/case1)/vector_store_ids]

A property name only appears in the required list, but not in the properties map - this is likely a typo; skipping this property. [context: foundIn=Components.Schemas.CreateAssistantRequest.tool_resourcesPayload.file_searchPayload.Case2Payload (#/components/schemas/CreateAssistantRequest/tool_resources/file_search/case2)/vector_stores]

Package version(s)

├── swift-openapi-generator<https://github.com/apple/[email protected]>
│   ├── swift-algorithms<https://github.com/apple/[email protected]>
│   │   └── swift-numerics<https://github.com/apple/[email protected]>
│   ├── swift-collections<https://github.com/apple/[email protected]>
│   ├── openapikit<https://github.com/mattpolzin/[email protected]>
│   │   └── yams<https://github.com/jpsim/[email protected]>
│   ├── yams<https://github.com/jpsim/[email protected]>
│   └── swift-argument-parser<https://github.com/apple/[email protected]>
├── swift-openapi-runtime<https://github.com/apple/[email protected]>
│   └── swift-http-types<https://github.com/apple/[email protected]>
└── swift-openapi-urlsession<https://github.com/apple/[email protected]>
    ├── swift-openapi-runtime<https://github.com/apple/[email protected]>
    │   └── swift-http-types<https://github.com/apple/[email protected]>
    ├── swift-http-types<https://github.com/apple/[email protected]>
    └── swift-collections<https://github.com/apple/[email protected]>

Expected behavior

The oneOf required definition should be properly recognized by the generator and used for the Swift output generation.

Environment

swift-driver version: 1.115.1 Apple Swift version 6.0.3 (swiftlang-6.0.3.1.10 clang-1600.0.30.1)
Target: arm64-apple-macosx15.0

Additional information

No response

@philippzagar philippzagar added kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue. labels Feb 19, 2025
@czechboy0
Copy link
Contributor

czechboy0 commented Feb 24, 2025

Hi @philippzagar,

that's right, the generator currently doesn't support "merging" JSON schemas from multiple places (in this instance, merging the required array into the parent object of the oneOf).

This won't be a trivial fix. If I'm reading it correctly, a simplified version of the above would be:

type: object
properties:
  a:
    type: string
  b:
    type: integer
oneOf:
  - required: [a]
  - required: [b]

In other words, it expresses that there's an object with properties a and b. And that either a or b (but not both) are required.

What Swift code would you want generated for this pattern?

@philippzagar
Copy link
Author

Hey @czechboy0,

thanks for the quick answer!

Yep, your minimal example is correct!
What I could envision is a (simplified) generated code like:

struct Object: Codable, Hashable, Sendable {
    var a: String?
    var b: Int?

    init(a: String, b: Int?) { self.a = a; self.b = b }
    init(a: String?, b: Int) { self.a = a; self.b = b }     // Parameter order might violate common Swift pattern of having optional parameters last
}

Of course, this isn't a perfect representation of the OpenAPI pattern in Swift, but I guess it's the best we'll get?

I imagine this might require some additional effort to get right, but it could be worthwhile to adjust the generator to at least not emit warnings (is there a non-ugly fix?), regardless of any changes to code generation.

philippzagar added a commit to StanfordSpezi/SpeziLLM that referenced this issue Feb 24, 2025
# OpenAPI spec preprocessing

## ♻️ Current situation & Problem
The OpenAI OpenAPI specification contains the following issues that
require preprocessing before code generation:

- **Incorrect `required` Property**: A non-existent property is
incorrectly marked as `required` (see
[`openai-openapi#421`](openai/openai-openapi#421)).
- **Unsupported `oneOf` Syntax**: The `swift-openapi-generator` does not
fully support `oneOf` with `required` properties (see
[`swift-openapi-generator#739`](apple/swift-openapi-generator#739)).
- **Deprecation Warnings**: `deprecated` markings in the OpenAPI spec
trigger warnings in the generated Swift code (see
[`swift-openapi-generator#106`](apple/swift-openapi-generator#106)
and
[`swift-openapi-generator#715`](apple/swift-openapi-generator#715)).

Without preprocessing, these issues result in unnecessary warnings
during the Swift code generation and in the resulting Swift client code.

## ⚙️ Release Notes 
- Add OpenAPI spec preprocessing script
- Preprocess OpenAPI OpenAI spec 


## 📚 Documentation
README for proper motivation for the need of preprocessing and usage
instructions.


## ✅ Testing
Local testing


## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).
@czechboy0
Copy link
Contributor

I imagine this might require some additional effort to get right, but it could be worthwhile to adjust the generator to at least not emit warnings (is there a non-ugly fix?), regardless of any changes to code generation.

Pragmatically, the easiest way to remove the warnings is to pre-process the OpenAPI doc and remove the oneOf. You'll then still end up with a usable generated struct with all optional properties.

What I could envision is a (simplified) generated code like:

struct Object: Codable, Hashable, Sendable {
    var a: String?
    var b: Int?

    init(a: String, b: Int?) { self.a = a; self.b = b }
    init(a: String?, b: Int) { self.a = a; self.b = b }     // Parameter order might violate common Swift pattern of having optional parameters last
}

All types generated today by Swift OpenAPI are usable both on the request and response side, so have to enforce rules both on serialization and deserialization paths. In the example above, the payload {} would happily decode into Object, but it's not a valid value for the JSON Schema.

This is a very reasonable feature request, but I'll also be honest by saying that it's non-trivial amount of work, and will require some creative design here 🙂

@czechboy0 czechboy0 added area/openapi Adding/updating a feature defined in OpenAPI. status/needs-design Needs further discussion and a concrete proposal. kind/enhancement Improvements to existing feature. and removed kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue. labels Feb 24, 2025
@philippzagar
Copy link
Author

philippzagar commented Feb 24, 2025

Pragmatically, the easiest way to remove the warnings is to pre-process the OpenAPI doc and remove the oneOf. You'll then still end up with a usable generated struct with all optional properties.

That's exactly what we're doing right now to prevent warnings being emitted. I understand that this the best pragmatic solution for now, but I feel that long-term the generator should support specs like the popular (and valid) OpenAI spec without emitting warnings :)

All types generated today by Swift OpenAPI are usable both on the request and response side, so have to enforce rules both on serialization and deserialization paths. In the example above, the payload {} would happily decode into Object, but it's not a valid value for the JSON Schema.

Thanks for the clarification and context here! I just laid out a very minimalistic envisioned Swift code, there will be some more complications along the way for sure!

This is a very reasonable feature request, but I'll also be honest by saying that it's non-trivial amount of work, and will require some creative design here 🙂

Thank you for looking into that! Will follow this issue closely! Please let me know if I can be of any help 🚀

@czechboy0
Copy link
Contributor

Please let me know if I can be of any help

If you'd like to investigate potential paths forward here, that'd be appreciated. It's unlikely we'll have time to invest into this in the short term. But I agree that long term, we'd like to support this feature.

I suspect the next steps are:

  1. Investigate how prominent this pattern of "mixed" schemas is, find more high-profile instances of it.
  2. Write a proposal of how to handle that in a generic and backwards-compatible way (mainly by prototyping what the desired generated code should be, updating the generator itself is usually the easier part).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/openapi Adding/updating a feature defined in OpenAPI. kind/enhancement Improvements to existing feature. status/needs-design Needs further discussion and a concrete proposal.
Projects
None yet
Development

No branches or pull requests

2 participants