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

Unify Configuration Management #241

Merged
merged 35 commits into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
156dd1d
Configuration PoC
jaceklyp Feb 10, 2023
0b0e725
More tests, and cleaner API
jaceklyp Feb 16, 2023
3dae118
Add more tests
jaceklyp Feb 18, 2023
5f3e07c
Add payload validation
jaceklyp Feb 18, 2023
4be949c
Get rid of ConfigurationTasks and add possibility to override urls in…
jaceklyp Feb 19, 2023
a1a13dd
Move code around
jaceklyp Feb 19, 2023
bf766e7
Move onDidStore to fetch method
jaceklyp Feb 19, 2023
0bb3ea5
Merge branch 'main' into jacek/unify-configuration
jaceklyp Feb 19, 2023
e240c13
APIRequest refactor part1
jaceklyp Feb 19, 2023
2261fe2
Continue refactoring
jaceklyp Feb 20, 2023
147a030
Use OptionSet instead of array of options
jaceklyp Feb 20, 2023
b78d5e2
Fix tests
jaceklyp Feb 20, 2023
2b4d58f
Add more tests
jaceklyp Feb 20, 2023
cf88a48
Create ConfigurationURLProvider to customize urls for Configuration
jaceklyp Feb 20, 2023
25006d4
Fix typo
jaceklyp Feb 20, 2023
2ebbd14
Provide setUserAgent method
jaceklyp Feb 20, 2023
c8242f9
Merge branch 'main' into jacek/unify-configuration
jaceklyp Feb 20, 2023
156654d
Fix deployment target
jaceklyp Feb 20, 2023
35c33d7
Fix compilation errors
jaceklyp Feb 20, 2023
0ccd2ff
Fix tests failing on broken locale
jaceklyp Feb 21, 2023
f3650bc
Error messages + handling 304
jaceklyp Feb 22, 2023
dfe4307
Fix for 304 handling logic
jaceklyp Feb 22, 2023
6bfae72
Update tests
jaceklyp Feb 22, 2023
a493f4c
Fix tests
jaceklyp Feb 22, 2023
4a37765
Fix naming
jaceklyp Feb 22, 2023
d379db2
Fix test
jaceklyp Feb 22, 2023
c971ba9
Address PR comments
jaceklyp Mar 6, 2023
e3ba7d9
Address PR comments #2
jaceklyp Mar 6, 2023
74333ef
Remove prints
jaceklyp Mar 6, 2023
219280d
Fix warnings and add logger option
jaceklyp Mar 6, 2023
c2a4ff1
Make Error public
jaceklyp Mar 6, 2023
54010a3
Add single configuration fetch method
jaceklyp Mar 7, 2023
2f3dee2
Get rid of fetch(any:) method
jaceklyp Mar 7, 2023
2f3bd90
Merge branch 'main' into jacek/unify-configuration
jaceklyp Mar 7, 2023
351d4d9
Fix compilation errors
jaceklyp Mar 7, 2023
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
42 changes: 37 additions & 5 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ let package = Package(
.library(name: "UserScript", targets: ["UserScript"]),
.library(name: "Crashes", targets: ["Crashes"]),
.library(name: "ContentBlocking", targets: ["ContentBlocking"]),
.library(name: "PrivacyDashboard", targets: ["PrivacyDashboard"]),
.library(name: "Configuration", targets: ["Configuration"]),
.library(name: "API", targets: ["API"]),
.library(name: "Navigation", targets: ["Navigation"]),
.library(name: "PrivacyDashboard", targets: ["PrivacyDashboard"])
],
dependencies: [
.package(name: "Autofill", url: "https://github.com/duckduckgo/duckduckgo-autofill.git", .exact("6.3.0")),
Expand Down Expand Up @@ -76,7 +78,7 @@ let package = Package(
name: "BloomFilter",
resources: [
.process("CMakeLists.txt")
]),
]),
.target(
name: "Crashes"
),
Expand Down Expand Up @@ -111,7 +113,7 @@ let package = Package(
]),
.target(
name: "UserScript"
),
),
.target(
name: "PrivacyDashboard",
dependencies: [
Expand All @@ -122,7 +124,24 @@ let package = Package(
.product(name: "PrivacyDashboardResources", package: "privacy-dashboard")
],
path: "Sources/PrivacyDashboard"
),
),
.target(
name: "Configuration",
dependencies: [
"API",
"BrowserServicesKit",
"Common"
]),
.target(
name: "API",
dependencies: [
"Common"
]),
.target(
name: "TestUtils",
dependencies: [
"API"
]),

// MARK: - Test targets
.testTarget(
Expand All @@ -139,6 +158,11 @@ let package = Package(
dependencies: [
"Common"
]),
.testTarget(
name: "APITests",
dependencies: [
"TestUtils"
]),
.testTarget(
name: "NavigationTests",
dependencies: [
Expand All @@ -160,7 +184,15 @@ let package = Package(
.testTarget(
name: "PersistenceTests",
dependencies: [
"Persistence"
"Persistence",
"TrackerRadarKit"
]
),
.testTarget(
name: "ConfigurationTests",
dependencies: [
"Configuration",
"TestUtils"
]
)
],
Expand Down
60 changes: 60 additions & 0 deletions Sources/API/APIHeaders.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
//
// APIHeaders.swift
// DuckDuckGo
//
// Copyright © 2023 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation

public typealias HTTPHeaders = [String: String]

public struct APIHeaders {

public typealias UserAgent = String
private static var userAgent: UserAgent?
public static func setUserAgent(_ userAgent: UserAgent) {
self.userAgent = userAgent
}

public init() { }

public var defaultHeaders: HTTPHeaders {
guard let userAgent = APIHeaders.userAgent else {
fatalError("Please set the userAgent before accessing defaultHeaders.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is time-sensitive, based on when setUserAgent is called, right? Shouldn't it rather be an assertion, to not crash the app on production?

}

let acceptEncoding = "gzip;q=1.0, compress;q=0.5"
let languages = Locale.preferredLanguages.prefix(6)
let acceptLanguage = languages.enumerated().map { index, language in
let q = 1.0 - (Double(index) * 0.1)
return "\(language);q=\(q)"
}.joined(separator: ", ")

return [
HTTPHeaderField.acceptEncoding: acceptEncoding,
HTTPHeaderField.acceptLanguage: acceptLanguage,
HTTPHeaderField.userAgent: userAgent
]
}

public func defaultHeaders(with etag: String?) -> HTTPHeaders {
guard let etag = etag else {
return defaultHeaders
}
return defaultHeaders.merging([HTTPHeaderField.ifNoneMatch: etag]) { (_, new) in new }
}

}
99 changes: 99 additions & 0 deletions Sources/API/APIRequest.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
//
// APIRequest.swift
// DuckDuckGo
//
// Copyright © 2023 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation

public typealias APIResponse = (data: Data?, response: HTTPURLResponse)
public typealias APIRequestCompletion = (APIResponse?, APIRequest.Error?) -> Void

public struct APIRequest {

private let request: URLRequest
private let urlSession: URLSession
private let requirements: APIResponseRequirements

public init<QueryParams: Collection>(configuration: APIRequest.Configuration<QueryParams>,
requirements: APIResponseRequirements = [],
urlSession: URLSession = .shared) {
self.request = configuration.request
self.requirements = requirements
self.urlSession = urlSession
}

@available(*, deprecated, message: "This method is deprecated. Please use the 'fetch()' async method instead.")
@discardableResult
public func fetch(completion: @escaping APIRequestCompletion) -> URLSessionDataTask {
let task = urlSession.dataTask(with: request) { (data, response, error) in
if let error = error {
completion(nil, .urlSession(error))
} else {
do {
let response = try self.validateAndUnwrap(data: data, response: response)
completion(response, nil)
} catch {
completion(nil, error as? APIRequest.Error ?? .urlSession(error))
}
}
}
task.resume()
return task
}

private func validateAndUnwrap(data: Data?, response: URLResponse?) throws -> APIResponse {
let httpResponse = try getHTTPResponse(from: response)

var data = data
if requirements.contains(.allow304), httpResponse.statusCode == HTTPURLResponse.Constants.notModifiedStatusCode {
data = nil // Avoid returning empty data
} else {
try httpResponse.assertSuccessfulStatusCode()
let data = data ?? Data()
if requirements.contains(.nonEmptyData), data.isEmpty {
throw APIRequest.Error.emptyData
}
}

if requirements.contains(.etag), httpResponse.etag == nil {
throw APIRequest.Error.missingEtagInResponse
}

return (data, httpResponse)
}

private func getHTTPResponse(from response: URLResponse?) throws -> HTTPURLResponse {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could remove this method and just define URLResponse.asHTTPURLResponse as throwable function.

guard let httpResponse = response?.asHTTPURLResponse else {
throw APIRequest.Error.invalidResponse
}
return httpResponse
}

public func fetch() async throws -> APIResponse {
let (data, response) = try await fetch(for: request)
return try validateAndUnwrap(data: data, response: response)
}

private func fetch(for request: URLRequest) async throws -> (Data, URLResponse) {
do {
return try await urlSession.data(for: request)
} catch let error {
throw Error.urlSession(error)
}
}

}
70 changes: 70 additions & 0 deletions Sources/API/APIRequestConfiguration.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
//
// APIRequestConfiguration.swift
// DuckDuckGo
//
// Copyright © 2023 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import Common

extension APIRequest {

public struct Configuration<QueryParams: Collection> where QueryParams.Element == (key: String, value: String) {

let url: URL
let method: HTTPMethod
let queryParameters: QueryParams
let allowedQueryReservedCharacters: CharacterSet?
let headers: HTTPHeaders
let body: Data?
let timeoutInterval: TimeInterval
let attribution: URLRequestAttribution?

public init(url: URL,
method: HTTPMethod = .get,
queryParameters: QueryParams = [],
allowedQueryReservedCharacters: CharacterSet? = nil,
headers: HTTPHeaders = APIHeaders().defaultHeaders,
body: Data? = nil,
timeoutInterval: TimeInterval = 60.0,
attribution: URLRequestAttribution? = nil) {
self.url = url
self.method = method
self.queryParameters = queryParameters
self.allowedQueryReservedCharacters = allowedQueryReservedCharacters
self.headers = headers
self.body = body
self.timeoutInterval = timeoutInterval
self.attribution = attribution
}

var request: URLRequest {
let url = url.appendingParameters(queryParameters, allowedReservedCharacters: allowedQueryReservedCharacters)
var request = URLRequest(url: url, timeoutInterval: timeoutInterval)
request.allHTTPHeaderFields = headers
request.httpMethod = method.rawValue
request.httpBody = body
if #available(iOS 15.0, macOS 12.0, *) {
if let attribution = attribution?.urlRequestAttribution {
request.attribution = attribution
}
}
return request
}

}

}
48 changes: 48 additions & 0 deletions Sources/API/APIRequestError.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
//
// APIRequestError.swift
// DuckDuckGo
//
// Copyright © 2023 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation

extension APIRequest {

public enum Error: Swift.Error, LocalizedError {

case urlSession(Swift.Error)
case invalidResponse
case missingEtagInResponse
case emptyData
case invalidStatusCode(Int)

public var errorDescription: String? {
switch self {
case .urlSession(let error):
return "URL session error: \(error.localizedDescription)"
case .invalidResponse:
return "Invalid response received."
case .missingEtagInResponse:
return "ETag header missing in response."
case .emptyData:
return "Empty data received in response."
case .invalidStatusCode(let statusCode):
return "Invalid status code received in response (\(statusCode))."
}
}
}

}
Loading