From f54c4f74e7884f7930560c08387817ce28271770 Mon Sep 17 00:00:00 2001 From: Bernhard Loibl Date: Mon, 7 Nov 2022 22:23:33 +0100 Subject: [PATCH] Preserve Query Order (#57) --- Package.resolved | 18 +++++++++++ Package.swift | 2 ++ Sources/URLRouting/Cookies.swift | 1 - Sources/URLRouting/Field.swift | 7 ++++- Sources/URLRouting/FormData.swift | 1 - .../URLRouting/Parsing/ParserPrinter.swift | 1 - .../URLRequestData+Foundation.swift | 12 ++++--- Sources/URLRouting/URLRequestData.swift | 31 +++++++++---------- Tests/URLRoutingTests/URLRoutingTests.swift | 7 ++++- 9 files changed, 53 insertions(+), 27 deletions(-) diff --git a/Package.resolved b/Package.resolved index 8c65d6834d..cc9ca29f39 100644 --- a/Package.resolved +++ b/Package.resolved @@ -28,6 +28,24 @@ "version": "0.8.1" } }, + { + "package": "swift-collections", + "repositoryURL": "https://github.com/apple/swift-collections", + "state": { + "branch": null, + "revision": "f504716c27d2e5d4144fa4794b12129301d17729", + "version": "1.0.3" + } + }, + { + "package": "SwiftDocCPlugin", + "repositoryURL": "https://github.com/apple/swift-docc-plugin", + "state": { + "branch": null, + "revision": "3303b164430d9a7055ba484c8ead67a52f7b74f6", + "version": "1.0.0" + } + }, { "package": "swift-parsing", "repositoryURL": "https://github.com/pointfreeco/swift-parsing", diff --git a/Package.swift b/Package.swift index ed101a3c79..159d1da303 100644 --- a/Package.swift +++ b/Package.swift @@ -15,6 +15,7 @@ let package = Package( ], dependencies: [ .package(url: "https://github.com/apple/swift-argument-parser", from: "0.5.0"), + .package(url: "https://github.com/apple/swift-collections", from: "1.0.3"), .package(url: "https://github.com/pointfreeco/swift-parsing", from: "0.10.0"), .package(url: "https://github.com/pointfreeco/xctest-dynamic-overlay", from: "0.3.0"), .package(name: "Benchmark", url: "https://github.com/google/swift-benchmark", from: "0.1.1"), @@ -23,6 +24,7 @@ let package = Package( .target( name: "URLRouting", dependencies: [ + .product(name: "OrderedCollections", package: "swift-collections"), .product(name: "Parsing", package: "swift-parsing"), .product(name: "XCTestDynamicOverlay", package: "xctest-dynamic-overlay"), ] diff --git a/Sources/URLRouting/Cookies.swift b/Sources/URLRouting/Cookies.swift index 490a8b412f..6b6e6fe17b 100644 --- a/Sources/URLRouting/Cookies.swift +++ b/Sources/URLRouting/Cookies.swift @@ -38,7 +38,6 @@ extension Cookies: ParserPrinter where Parsers: ParserPrinter { input.headers["cookie", default: []].prepend( cookies - .sorted(by: { $0.key < $1.key }) .flatMap { name, values in values.map { "\(name)=\($0 ?? "")" } } .joined(separator: "; ")[...] ) diff --git a/Sources/URLRouting/Field.swift b/Sources/URLRouting/Field.swift index 8ba53ac3a5..167524caec 100644 --- a/Sources/URLRouting/Field.swift +++ b/Sources/URLRouting/Field.swift @@ -103,6 +103,11 @@ extension Field: ParserPrinter where Value: ParserPrinter { @inlinable public func print(_ output: Value.Output, into input: inout URLRequestData.Fields) rethrows { if let defaultValue = self.defaultValue, isEqual(output, defaultValue) { return } - input[self.name, default: []].prepend(try valueParser.print(output)) + try input.fields.updateValue( + forKey: input.isNameCaseSensitive ? self.name : self.name.lowercased(), + insertingDefault: [], + at: 0, + with: { $0.prepend(try self.valueParser.print(output)) } + ) } } diff --git a/Sources/URLRouting/FormData.swift b/Sources/URLRouting/FormData.swift index 12d4213687..f776dccfc0 100644 --- a/Sources/URLRouting/FormData.swift +++ b/Sources/URLRouting/FormData.swift @@ -46,7 +46,6 @@ extension Data { init(encoding fields: URLRequestData.Fields) { self.init( fields - .sorted(by: { $0.key < $1.key }) .flatMap { pair -> [String] in let (name, values) = pair guard let name = name.addingPercentEncoding(withAllowedCharacters: .urlQueryParamAllowed) diff --git a/Sources/URLRouting/Parsing/ParserPrinter.swift b/Sources/URLRouting/Parsing/ParserPrinter.swift index fd24fdd85a..0b5ef9175a 100644 --- a/Sources/URLRouting/Parsing/ParserPrinter.swift +++ b/Sources/URLRouting/Parsing/ParserPrinter.swift @@ -66,7 +66,6 @@ extension ParserPrinter where Input == URLRequestData { components.path = "/\(data.path.joined(separator: "/"))" if !data.query.isEmpty { components.queryItems = data.query - .sorted(by: { $0.key < $1.key }) .flatMap { name, values in values.map { URLQueryItem(name: name, value: $0.map(String.init)) } } diff --git a/Sources/URLRouting/URLRequestData+Foundation.swift b/Sources/URLRouting/URLRequestData+Foundation.swift index 5d52e81e8b..f93aaeb252 100644 --- a/Sources/URLRouting/URLRequestData+Foundation.swift +++ b/Sources/URLRouting/URLRequestData+Foundation.swift @@ -36,9 +36,12 @@ extension URLRequestData { query[item.name, default: []].append(item.value) } ?? [:], fragment: components.fragment, - headers: request.allHTTPHeaderFields?.mapValues { - $0.split(separator: ",", omittingEmptySubsequences: false).map { String($0) } - } ?? [:], + headers: .init( + request.allHTTPHeaderFields?.map { key, value in + (key, value.split(separator: ",", omittingEmptySubsequences: false).map { String($0) }) + } ?? [], + uniquingKeysWith: { $1 } + ), body: request.httpBody ) } @@ -78,7 +81,6 @@ extension URLComponents { self.path = "/\(data.path.joined(separator: "/"))" if !data.query.isEmpty { self.queryItems = data.query - .sorted(by: { $0.key < $1.key }) .flatMap { name, values in values.map { URLQueryItem(name: name, value: $0.map(String.init)) } } @@ -103,7 +105,7 @@ extension URLRequest { guard let url = URLComponents(data: data).url else { return nil } self.init(url: url) self.httpMethod = data.method - for (name, values) in data.headers.sorted(by: { $0.key < $1.key }) { + for (name, values) in data.headers { for value in values { if let value = value { self.addValue(String(value), forHTTPHeaderField: name) diff --git a/Sources/URLRouting/URLRequestData.swift b/Sources/URLRouting/URLRequestData.swift index 48727d9d6d..5354a3964a 100644 --- a/Sources/URLRouting/URLRequestData.swift +++ b/Sources/URLRouting/URLRequestData.swift @@ -1,4 +1,5 @@ import Foundation +import OrderedCollections /// A parseable URL request. /// @@ -76,9 +77,9 @@ public struct URLRequestData: Equatable, _EmptyInitializable { host: String? = nil, port: Int? = nil, path: String = "", - query: [String: [String?]] = [:], + query: OrderedDictionary = [:], fragment: String? = nil, - headers: [String: [String?]] = [:], + headers: OrderedDictionary = [:], body: Data? = nil ) { self.body = body @@ -99,13 +100,13 @@ public struct URLRequestData: Equatable, _EmptyInitializable { /// Used by ``URLRequestData`` to model query parameters and headers in a way that can be /// efficiently parsed. public struct Fields { - public var fields: [String: ArraySlice] + public var fields: OrderedDictionary> @usableFromInline var isNameCaseSensitive: Bool @inlinable public init( - _ fields: [String: ArraySlice] = [:], + _ fields: OrderedDictionary> = [:], isNameCaseSensitive: Bool ) { self.fields = [:] @@ -152,9 +153,9 @@ extension URLRequestData: Codable { host: try container.decodeIfPresent(String.self, forKey: .host), port: try container.decodeIfPresent(Int.self, forKey: .port), path: try container.decodeIfPresent(String.self, forKey: .path) ?? "", - query: try container.decodeIfPresent([String: [String?]].self, forKey: .query) ?? [:], + query: try container.decodeIfPresent(OrderedDictionary.self, forKey: .query) ?? [:], fragment: try container.decodeIfPresent(String.self, forKey: .fragment), - headers: try container.decodeIfPresent([String: [String?]].self, forKey: .headers) ?? [:], + headers: try container.decodeIfPresent(OrderedDictionary.self, forKey: .headers) ?? [:], body: try container.decodeIfPresent(Data.self, forKey: .body) ) } @@ -219,27 +220,27 @@ extension URLRequestData: Hashable { } extension URLRequestData.Fields: Collection { - public typealias Element = Dictionary>.Element - public typealias Index = Dictionary>.Index + public typealias Element = OrderedDictionary>.Element + public typealias Index = OrderedDictionary>.Index @inlinable public var startIndex: Index { - self.fields.startIndex + self.fields.elements.startIndex } @inlinable public var endIndex: Index { - self.fields.endIndex + self.fields.elements.endIndex } @inlinable public subscript(position: Index) -> Element { - self.fields[position] + self.fields.elements[position] } @inlinable public func index(after i: Index) -> Index { - self.fields.index(after: i) + self.fields.elements.index(after: i) } } @@ -253,11 +254,7 @@ extension URLRequestData.Fields: ExpressibleByDictionaryLiteral { extension URLRequestData.Fields: Equatable { @inlinable public static func == (lhs: Self, rhs: Self) -> Bool { - guard lhs.count == rhs.count else { return false } - for key in lhs.fields.keys { - guard lhs[key] == rhs[key] else { return false } - } - return true + lhs.fields == rhs.fields } } diff --git a/Tests/URLRoutingTests/URLRoutingTests.swift b/Tests/URLRoutingTests/URLRoutingTests.swift index 71785135f3..a11dfdcd2c 100644 --- a/Tests/URLRoutingTests/URLRoutingTests.swift +++ b/Tests/URLRoutingTests/URLRoutingTests.swift @@ -79,6 +79,11 @@ class URLRoutingTests: XCTestCase { XCTAssertEqual("Blob", name) XCTAssertEqual(42, age) XCTAssertEqual(["debug": ["1"]], request.query) + + XCTAssertEqual( + try p.print(("Blob", 42)), + URLRequestData(query: ["name": ["Blob"], "age": ["42"]]) + ) } func testQueryDefault() throws { @@ -190,7 +195,7 @@ class URLRoutingTests: XCTestCase { try p.parse(&request) ) XCTAssertEqual( - URLRequestData(headers: ["cookie": ["isAdmin=true; userId=42"]]), + URLRequestData(headers: ["cookie": ["userId=42; isAdmin=true"]]), try p.print(Session(userId: 42, isAdmin: true)) ) }