Skip to content

Create a URI encoder and decoder that works with Codable types #192

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

Closed
czechboy0 opened this issue Aug 12, 2023 · 6 comments · Fixed by apple/swift-openapi-runtime#41 or apple/swift-openapi-runtime#44
Assignees
Labels
area/runtime Affects: the runtime library. kind/feature New feature. size/M Medium task. (A couple of days of work.)
Milestone

Comments

@czechboy0
Copy link
Contributor

czechboy0 commented Aug 12, 2023

Create a URI encoder and decoder that works with Codable types.

More details in this comment: #182 (comment)

@czechboy0 czechboy0 added area/runtime Affects: the runtime library. kind/feature New feature. size/M Medium task. (A couple of days of work.) labels Aug 12, 2023
@bfrearson
Copy link
Contributor

This is definitely needed, but before we continue it would be good to reach a consensus on what we should do with a nested type such as:

struct NestedType: Encodable {
    let name: String
    let ship: Ship
}

struct Ship: Encodable {
    let designation: String
    let crew: [String]
    let type: String
}

The expected output of our encoder, I assume, would always be key-value pairs in the form key1=value1&key2=value2. There is nothing in the openapi spec that prevents a schema like the example above from being used here, even though it doesn't match the architecture, so we need to have a way of handling this, even if it is bad practice.

There's multiple ways that we could handle these types, and in the long run it would be good to support a few different methods. However, I think for the moment, we should decide on a single implementation and expand from there.

What should the output of our encoder look like for a type that itself contains other Codable types like the example above (percent encoding omitted for clarity)?

  • dot syntax: name=Han Solo&ship.designation=Millennium Falcon&ship.crew=Chewie, Leia, C3PO&ship.type=YT-1300F light freighter
  • JSON serialized strings for lower-level data: name=Han Solo&ship={crew = ( "Chewie, Leia, C3PO"); designation = "Millenium Falcon"; type = "YT-1300F light freighter";}
  • error: "url encoded forms must use a flat data structure"
  • Something else?

For the very short term, I'd suggest we error out until the initial encoder is written, but I imagine that's not acceptable in the longer run.

@czechboy0
Copy link
Contributor Author

I'd like us to support any arbitrarily nested Codable type, as it's not going to be that much more work than just top level items. I have a design in mind, let me try to put together a prototype end of this or next week and we can take it from there.

@czechboy0
Copy link
Contributor Author

Early encoder prototype I'm working on: apple/swift-openapi-runtime#41

@czechboy0
Copy link
Contributor Author

So, after actually reading the 6570 and 1866 RFCs, turns out nesting containers is not a supported feature. All the nesting approaches seem to be conventions that people invented on top.

The first implementation of URIEncoder/URIDecoder will not support nesting containers, but I'd be open to enhancing it in the future to support nesting, assuming we can have some confidence that what we implement is actually used widely.

@bfrearson
Copy link
Contributor

I think this is a reasonable stance. Any well formed api will adhere to the standards, and users in need of a more complex data structure should probably be using json instead.

The only pain point is that the open api spec explicitly doesn't enforce this conformance, so it's possible to define an openapi document that will try to use a nested object. What's the error state if we try and do this? Should the generator fail with an error here?

@czechboy0
Copy link
Contributor Author

The URI coder itself will fail at runtime with a descriptive error. I'm afraid detecting this in the generator might be a lot of work and doesn't bring much value. Users won't get this working anyway.

@czechboy0 czechboy0 changed the title Create a URL form encoder and decoder that works with Codable types Create a URI encoder and decoder that works with Codable types Aug 24, 2023
@czechboy0 czechboy0 added this to the 0.1.next milestone Aug 25, 2023
czechboy0 added a commit to apple/swift-openapi-runtime that referenced this issue Aug 29, 2023
Introduce URIEncoder and URIDecoder types

### Motivation

Fixes apple/swift-openapi-generator#192.

For refactoring how we encode:
- path parameters
- query parameters
- headers
- (new feature, coming later) `application/x-www-form-urlencoded` bodies

Supports:
- form + explode
- form + unexplode
- simple + explode
- simple + unexplode
- form where space is encoded as + instead of %20 (for bodies) + explode
- form where space is encoded as + instead of %20 (for bodies) + unexplode

### Modifications

First step - introduce two new types: `URIEncoder` and `URIDecoder`.

They're configurable types that can handle the URI Template (RFC 6570) form and simple styles, refined by OpenAPI 3.0.3, and also the `application/x-www-form-urlencoded` format (mostly identical to URI Template).

### Result

The types can be used now, subsequent PRs will integrate them.

### Test Plan

Added unit tests for the individual parts of the coder, but also roundtrip tests.


Reviewed by: glbrntt

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. 

#41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/runtime Affects: the runtime library. kind/feature New feature. size/M Medium task. (A couple of days of work.)
Projects
None yet
2 participants