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

Detect invalid use of binary properties in JSON object schemas #701

Open
MahdiBM opened this issue Dec 13, 2024 · 8 comments
Open

Detect invalid use of binary properties in JSON object schemas #701

MahdiBM opened this issue Dec 13, 2024 · 8 comments
Labels
kind/enhancement Improvements to existing feature.
Milestone

Comments

@MahdiBM
Copy link
Contributor

MahdiBM commented Dec 13, 2024

Description

Compiler error for generated code related to binary content encoding.
See below for a quick repro.

the error is:

API/Sources/OpenAPI/GeneratedSources/Types.swift:27:24: error: type 'Components.Schemas.CategoryResponse' does not conform to protocol 'Decodable'
25 |     package enum Schemas {
26 |         /// - Remark: Generated from `#/components/schemas/CategoryResponse`.
27 |         package struct CategoryResponse: Codable, Hashable, Sendable {
   |                        `- error: type 'Components.Schemas.CategoryResponse' does not conform to protocol 'Decodable'
28 |             /// The image file for the category (supported formats: jpg, png, webp)
29 |             ///
30 |             /// - Remark: Generated from `#/components/schemas/CategoryResponse/image`.
31 |             package var image: OpenAPIRuntime.HTTPBody?
   |                         `- note: cannot automatically synthesize 'Decodable' because 'OpenAPIRuntime.HTTPBody?' does not conform to 'Decodable'
32 |             /// Creates a new `CategoryResponse`.
33 |             ///

Swift.Decodable:2:5: note: protocol requires initializer 'init(from:)' with type 'Decodable'
1 | public protocol Decodable {
2 |     init(from decoder: any Decoder) throws
  |     `- note: protocol requires initializer 'init(from:)' with type 'Decodable'
3 | }

API/Sources/OpenAPI/GeneratedSources/Types.swift:27:24: error: type 'Components.Schemas.CategoryResponse' does not conform to protocol 'Encodable'
25 |     package enum Schemas {
26 |         /// - Remark: Generated from `#/components/schemas/CategoryResponse`.
27 |         package struct CategoryResponse: Codable, Hashable, Sendable {
   |                        `- error: type 'Components.Schemas.CategoryResponse' does not conform to protocol 'Encodable'
28 |             /// The image file for the category (supported formats: jpg, png, webp)
29 |             ///
30 |             /// - Remark: Generated from `#/components/schemas/CategoryResponse/image`.
31 |             package var image: OpenAPIRuntime.HTTPBody?
   |                         `- note: cannot automatically synthesize 'Encodable' because 'OpenAPIRuntime.HTTPBody?' does not conform to 'Encodable'
32 |             /// Creates a new `CategoryResponse`.
33 |             ///

Swift.Encodable:2:10: note: protocol requires function 'encode(to:)' with type 'Encodable'
1 | public protocol Encodable {
2 |     func encode(to encoder: any Encoder) throws
  |          `- note: protocol requires function 'encode(to:)' with type 'Encodable'
3 | }

Reproduction

openapi: "3.1.0"
info:
  title: My API
  version: 1.0.0
components:
  schemas:
    CategoryResponse:
      type: object
      properties:
        image:
          title: Image
          type: string
          contentEncoding: binary
          description: "The image file for the category (supported formats: jpg, png, webp)"

Package version(s)

generator 1.3.0
runtime 1.5.0

Expected behavior

code should compile

Environment

irrelevant but Swift 6, M-series mac.

Additional information

No response

@MahdiBM MahdiBM added kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue. labels Dec 13, 2024
@czechboy0
Copy link
Contributor

Discussed offline, examples of how to write multipart requests/responses that include raw bytes and also JSON are here: https://swiftpackageindex.com/apple/swift-openapi-generator/1.5.1/documentation/swift-openapi-generator/soar-0009#Describing-a-multipart-request-in-OpenAPI

No bug identified in the generator here, closing.

@czechboy0 czechboy0 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 16, 2024
@MahdiBM
Copy link
Contributor Author

MahdiBM commented Dec 16, 2024

So ... we were holding it wrong ... but the generator also did happily produce non-compiling code ... isn't that considered a bug?
I thought the generator should refuse to produce any code at some point that it can notice something is wrong?

@czechboy0
Copy link
Contributor

The generator strives to always generate valid Swift code for valid OpenAPI documents. We do want to get as close to 100% as possible on that metric.

That's separate from always erroring out on invalid OpenAPI documents, I'd consider that "best effort", but doing that very reliably would require some very sophisticated analysis of the OpenAPI document that we currently don't have, nor does OpenAPIKit provide to us.

That said - if you identify a specific scenario that's relatively easy to detect in the generator and always leads to non-compiling code, please do file an issue (or even better, a PR). But it really depends on how much complexity detecting the scenario will add to the generator. If it's relatively localized, then we're likely to take it, but if it requires global knowledge, we might not (based on our third principle).

@simonjbeaumont
Copy link
Collaborator

@MahdiBM we do some validation (in addition to OpenAPIKit) here:

func validateDoc(_ doc: ParsedOpenAPIRepresentation, config: Config) throws -> [Diagnostic] {
, which is where we would add this, if we wanted it.

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Dec 16, 2024

I mean, it doesn't really affect me anymore since I've already encountered this problem and know what to do to resolve it, but it looks to me that the issue should stay open unless the generator is happy with the non-compiling code?

I know you're trying to keep the issues list clean, but a valid issue is a valid issue 🤷‍♂️.

This issue also very clearly describes one of the situations that result in non-compiling code. The yaml code above is enough to trigger this behavior. IIRC i did try that exact minified yaml file before filing this issue.

@czechboy0 czechboy0 changed the title Compiler error for generated code related to binary content encoding Detect invalid use of binary properties in JSON object schemas Dec 16, 2024
@czechboy0 czechboy0 reopened this Dec 16, 2024
@czechboy0
Copy link
Contributor

Sounds good, I've reopened and renamed the issue to track the exact case you reported. A schema that uses a binary property, without a multipart operation using it. That we can investigate emitting a diagnostic for.

@czechboy0
Copy link
Contributor

Just to confirm - you didn't get any diagnostic from the generated already, right? No warnings emitted during the build?

@czechboy0 czechboy0 added kind/enhancement Improvements to existing feature. and removed status/triage Collecting information required to triage the issue. kind/bug Feature doesn't work as expected. labels Dec 16, 2024
@czechboy0 czechboy0 added this to the Post-1.0 milestone Dec 16, 2024
@MahdiBM
Copy link
Contributor Author

MahdiBM commented Dec 16, 2024

Can't properly recall off-hand but i do think I didn't get any warnings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements to existing feature.
Projects
None yet
Development

No branches or pull requests

3 participants