Skip to content

Conversation

czechboy0
Copy link
Contributor

@czechboy0 czechboy0 commented Aug 10, 2023

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.

Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a pretty sensible shape. Not sure about the ordering of quality and contentType though.

@czechboy0
Copy link
Contributor Author

@gjcairo @glbrntt if you have more time, would love your thoughts on the associated changes in the runtime library: apple/swift-openapi-runtime#37

Copy link
Collaborator

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Honza!

@czechboy0 czechboy0 changed the title [Prototype] Accept header [Generator] Accept header Aug 23, 2023
@czechboy0 czechboy0 marked this pull request as ready for review August 23, 2023 11:55
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

czechboy0 added a commit to apple/swift-openapi-runtime that referenced this pull request Aug 24, 2023
[Runtime] Accept header

### Motivation

SOAR-0003 was accepted, this is the runtime side of the implementation.

### Modifications

- Introduced a new protocol `AcceptableProtocol`, which all the generated, operation-specific Accept enums conform to.
- Introduced a new struct `QualityValue`, which wraps the quality parameter of the content type. Since the precision is capped at 3 decimal places, the internal storage is in 1000's, allowing for more reliable comparisons, as floating point numbers are only used when serialized into headers.
- Introduced a new struct `AcceptHeaderContentType`, generic over `AcceptableProtocol`, which adds `QualityValue` to each generated Accept enum.
- Introduced new extensions on `Converter` that allow setting and getting the Accept header.

### Result

These are the requirements for apple/swift-openapi-generator#185.

### Test Plan

Added unit tests for both `QualityValue` and `AcceptHeaderContentType`, and for the new `Converter` methods.


Reviewed by: gjcairo

Builds:
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (api breakage) - 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. 

#37
@czechboy0 czechboy0 merged commit 0bebccd into apple:main Aug 24, 2023
@czechboy0 czechboy0 deleted the hd-accept-header-prototype branch August 24, 2023 13:15
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Accept header information to the Input struct

4 participants