Skip to content

Conversation

@czechboy0
Copy link
Contributor

@czechboy0 czechboy0 commented Nov 7, 2023

Motivation

Implement SOAR-0009. Fixes #36.

Modifications

It's a large diff, and adds some complexity, however I tried to avoid needlessly complicating existing code and opted for duplicating parts of the code specifically for multipart, for easier reasoning.

The implementation very much maps to the changes described in the proposal, which I won't repeat here.

Some notable highlights before you dive in:

  • Testing is mainly done through snippet tests, and I added one request and one response operation to make sure things work end to end. Maybe review those first.
  • The logic for generating request and response bodies got slightly extended to detect when we're generating multipart content, and branches off to bespoke code specifically for multipart, all in the new Multipart directory.
  • Made a few related changes, but tried to keep this isolated to multipart as much as reasonable.
  • Let me know which parts I should elaborate on, I'm happy to explain in more detail.

Result

Multipart content now works, as proposed in SOAR-0009.

Test Plan

Added a bunch of snippet tests and two new file-based reference test operations, including petstore consumer tests to verify that it all works at runtime as well.

@czechboy0 czechboy0 changed the title [WIP] [Generator] Multipart support [Generator] Multipart support Nov 24, 2023
@czechboy0 czechboy0 marked this pull request as ready for review November 24, 2023 11:56
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.

👏 Great stuff.

@czechboy0 czechboy0 enabled auto-merge (squash) November 24, 2023 16:16
@czechboy0 czechboy0 merged commit ad4060b into apple:main Nov 24, 2023
@czechboy0 czechboy0 deleted the hd-multipart branch November 24, 2023 17:23
@czechboy0 czechboy0 added the ⚠️ semver/major Breaks existing public API. label Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ semver/major Breaks existing public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support the multipart content type

2 participants