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.

Sign up for GitHub

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

Jump to bottom

Add HTTPClient for Web API Client Library #64

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add HTTPClient for Web API Client Library #64

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b13acf5
add HTTPClient
zunda-pixel Aug 9, 2024
f62204f
add HTTPClientFoundation
zunda-pixel Aug 13, 2024
d782d27
combining Body and Data into one associatedtype.
zunda-pixel Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ let package = Package(
"HTTPTypes",
]
),
.target(
name: "HTTPClient",
dependencies: [
.target(name: "HTTPTypes"),
]
),
.target(
name: "HTTPClientFoundation",
dependencies: [
.target(name: "HTTPClient"),
]
),
.testTarget(
name: "HTTPTypesTests",
dependencies: [
Expand Down
8 changes: 8 additions & 0 deletions Sources/HTTPClient/HTTPClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import Foundation
import HTTPTypes

public protocol HTTPClientProtocol {
associatedtype Body
associatedtype Data
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add associatedType Body, Data for Data, ByteBuffer, [Int8].
#15 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I think having associated types here is kinda defeating the purpose of having a general protocol level abstraction for an HTTP client. In my opinion, we should come up with standardised types for the type of data.

Furthermore, I see two more outstanding questions:

  1. We should support optional trailers on both the request and response side
  2. We should support bi-direction body streaming

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we adopt 1 or 2?

  1. one associatedType(Data), data, body has same Type.
  2. no associatedType, use Foundation.Data

Copy link
Member

@FranzBusch FranzBusch Aug 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should adopt any of the two. There is currently no "bag-of-bytes" currency type that is used across the ecosystems. Choosing either Data or ByteBuffer will come with downsides.

In my opinion, we have to solve this question in the standard library first before we can tackle providing a common API abstraction here.

Copy link
Contributor Author

@zunda-pixel zunda-pixel Aug 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can we adopt Foundation.Data without waiting for the issue to be resolved?
I think solving the standard library issue is difficult and will take too much time.

Does Apple/SwiftLang have any plans to resolve this?

Copy link
Member

Choose a reason for hiding this comment

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

Can we adopt Foundation.Data without waiting for the issue to be resolved?

We can't adopt Foundation.Data across the ecosystem right now since the server ecosystem uses ByteBuffer and is built around that. Adopting Foundation.Data would incur unnecessary allocations at the API boundaries to bridge between the types.

Does Apple/SwiftLang have any plans to resolve this?

There were previous discussions around this topic but there currently is no proposed solution. One of the focus areas of the Swift project right now is tackling ownership and lifetime in the standard library including in collections like Array<UInt8>. This might inform some of the next steps around a common bag-of-bytes type.

Sadly, I think this puts the proposal with creating a shared API abstraction for HTTP clients on hold right now. Until we can solve the bag-of-bytes problem at the language level. Now you can still create this protocol in your own code and conform the various clients to it.

func execute(for request: HTTPRequest, from body: Body?) async throws -> (Data, HTTPResponse)
}
30 changes: 30 additions & 0 deletions Sources/HTTPClientFoundation/URLSession++HTTPClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import Foundation
import HTTPClient
import HTTPTypes
import HTTPTypesFoundation

#if canImport(FoundationNetworking)
import FoundationNetworking
#endif

#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS) || os(visionOS) || compiler(>=6)
@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
extension URLSession: HTTPClientProtocol {
public func execute(
for request: HTTPTypes.HTTPRequest,
from body: Foundation.Data?
) async throws -> (Foundation.Data, HTTPTypes.HTTPResponse) {
if let body {
try await self.upload(for: request, from: body)
} else {
try await self.data(for: request)
}
}
}

extension HTTPClientProtocol where Self == URLSession {
public static func urlSession(_ urlSession: Self) -> Self {
return urlSession
}
}
#endif