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

Add Accept header information to the Input struct #160

Closed
czechboy0 opened this issue Aug 1, 2023 · 2 comments · Fixed by #185
Closed

Add Accept header information to the Input struct #160

czechboy0 opened this issue Aug 1, 2023 · 2 comments · Fixed by #185
Assignees
Labels
area/generator Affects: plugin, CLI, config file. kind/feature New feature. status/needs-design Needs further discussion and a concrete proposal.

Comments

@czechboy0
Copy link
Contributor

This is a follow-up task to #6.

  • Synthesizing an Input parameter that's a type-safe variant of the "accept" header
  • The synthesized accept enum should only contain supported content types that we actually generate Output cases for

This way, the server implementer, conforming their type to APIProtocol, will have a clear signal from the user about which content type they want.

Things to keep in mind:

  • Client might not care
  • Client might specify */*
  • Client might specify application/*
  • Client might specify multiple, and use priorities (the q parameter)

The synthesized enum might not be concrete types, but some representation like:

enum Accept {
  case any
  case anySubtype(String)

  enum SupportedContentTypes {
    case json
    case yaml
  }

  case concrete(SupportedContentTypes)

  // The order here is significant, sorted from most to least desired, according to the rules in https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept
  case multiple([SupportedContentTypes]
}

This will require a proposal, as there are many options and it needs to be carefully considered.

@czechboy0 czechboy0 added area/generator Affects: plugin, CLI, config file. status/needs-design Needs further discussion and a concrete proposal. kind/feature New feature. labels Aug 1, 2023
@czechboy0 czechboy0 self-assigned this Aug 10, 2023
@czechboy0 czechboy0 changed the title Provide Accept header information to the server handler Add Accept header information to the Input struct Aug 10, 2023
@czechboy0
Copy link
Contributor Author

Was the subject of SOAR-0003: #201.

czechboy0 added a commit that referenced this issue Aug 24, 2023
[Generator] Accept header

### Motivation

Fixes #160.

SOAR-0003 was accepted, this is the generator side of the implementation. The runtime side, which must land first, is at: apple/swift-openapi-runtime#37.

### Modifications

- Adapted the generator logic for the changes - see the file-based reference tests for concrete examples of what the generated `AcceptableContentType` enum looks like.
- Introduced `translateRawRepresentableEnum`, which allows sharing logic between generating this new enum and other string-based enums.
- Explicitly skip parameters that match reserved headers, as dictated by the specification.

### Result

SOAR-0003 as proposed, working behind the `multipleContentTypes` feature flag.

### Test Plan

Adapted PetstoreConsumerTests to test the new behavior, adapter reference tests.


Reviewed by: gjcairo, glbrntt

Builds:
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (docc test) - Build finished. 
     ✔︎ pull request validation (integration test) - Build finished. 
     ✔︎ pull request validation (nightly) - Build finished. 
     ✔︎ pull request validation (soundness) - Build finished. 

#185
@czechboy0
Copy link
Contributor Author

Will land in 0.1.11 behind the feature flag multipleContentTypes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/generator Affects: plugin, CLI, config file. kind/feature New feature. status/needs-design Needs further discussion and a concrete proposal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant