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

Adopt HTTPRequest and HTTPResponse type from swift-http-types #567

Open
grdsdev opened this issue Oct 14, 2024 · 2 comments · May be fixed by #568
Open

Adopt HTTPRequest and HTTPResponse type from swift-http-types #567

grdsdev opened this issue Oct 14, 2024 · 2 comments · May be fixed by #568
Labels
enhancement New feature or request
Milestone

Comments

@grdsdev
Copy link
Collaborator

grdsdev commented Oct 14, 2024

Request for Change

The swift-http-types package was added by @zunda-pixel in the #564 which refactored the internal HTTPHeaders type to use the HTTPFields from the package.

The swift-http-types also provides both HTTPRequest and HTTPResponse generic types.

Current behavior

We currently have both HTTPRequest and HTTPResponse types that we use internally for all requests/responses.

New behavior

Refactor internal HTTP layer to use types from swift-http-types package.

Open questions

swift-http-types doesn't standardize the body property, that is up to us to define how we're handling it.

I suggest the refactor of the HTTPClientType to be:

func send(for request: HTTPRequest, body: Data?) async throws -> (Data, HTTPResponse)

And forward to upload(from:body:) in case a non-nil body is provided, or data(for:), as:

func send(for request: HTTPRequest, body: Data?) async throws -> (Data, HTTPResponse) {
  let (data, response) = if let body {
    try await session.upload(for: request, body: body)
  } else {
    try await session.data(for: request)
  }

  let httpResponse = HTTPResponse(response as! HTTPURLResponse)
  return (data, httpResponse)
}
@grdsdev grdsdev added the enhancement New feature or request label Oct 14, 2024
@grdsdev grdsdev added this to the v3 milestone Oct 14, 2024
@zunda-pixel
Copy link
Contributor

zunda-pixel commented Oct 15, 2024

I think that adding body(Data, ByteBuffer or [UInt8]) to HTTPRequest is difficult problem on Server Side and Client Side.
And I think func send(for request: HTTPRequest, from body: Data?) async throws -> (Data, HTTPResponse) is good!

@zunda-pixel
Copy link
Contributor

zunda-pixel commented Oct 15, 2024

I created pr #568

@grdsdev grdsdev linked a pull request Oct 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants