Skip to content

Commit 5266f86

Browse files
committed
Handle paginated registry metadata responses
In [4.1 List Package Releases](https://github.com/swiftlang/swift-package-manager/blob/main/Documentation/PackageRegistry/Registry.md#41-list-package-releases) it is stated that a server may respond with a `Link` header that contains a pointer to a subsequent page of results. SPM is not checking for this link in the `Link` header and so if a registry returns paginated results only the first page of versions is searched when resolving. Respect the `next` link in the `Link` header by loading the next page of results and building up a list of versions, continuing until there is no `next` link present in the `Link` header of the last result. Issue: #8215
1 parent 0340bb1 commit 5266f86

File tree

3 files changed

+168
-77
lines changed

3 files changed

+168
-77
lines changed

Sources/PackageMetadata/PackageMetadata.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ public struct PackageSearchClient {
352352
) { result in
353353
do {
354354
let metadata = try result.get()
355-
let alternateLocations = metadata.alternateLocations ?? []
355+
let alternateLocations = metadata.alternateLocations
356356
return completion(.success(Set(alternateLocations)))
357357
} catch {
358358
return completion(.failure(error))

Sources/PackageRegistry/RegistryClient.swift

Lines changed: 166 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -185,14 +185,16 @@ public final class RegistryClient: Cancellable {
185185
observabilityScope.emit(debug: "registry for \(package): \(registry)")
186186

187187
let underlying = {
188-
self._getPackageMetadata(
189-
registry: registry,
190-
package: registryIdentity,
191-
timeout: timeout,
192-
observabilityScope: observabilityScope,
193-
callbackQueue: callbackQueue,
194-
completion: completion
195-
)
188+
_ = Task {
189+
let result = await self._getPackageMetadata(
190+
registry: registry,
191+
package: registryIdentity,
192+
timeout: timeout,
193+
observabilityScope: observabilityScope,
194+
callbackQueue: callbackQueue
195+
)
196+
completion(result)
197+
}
196198
}
197199

198200
if registry.supportsAvailability {
@@ -217,64 +219,106 @@ public final class RegistryClient: Cancellable {
217219
package: PackageIdentity.RegistryIdentity,
218220
timeout: DispatchTimeInterval?,
219221
observabilityScope: ObservabilityScope,
220-
callbackQueue: DispatchQueue,
221-
completion: @escaping (Result<PackageMetadata, Error>) -> Void
222-
) {
223-
let completion = self.makeAsync(completion, on: callbackQueue)
224-
222+
callbackQueue: DispatchQueue
223+
) async -> Result<PackageMetadata, Error> {
225224
guard var components = URLComponents(url: registry.url, resolvingAgainstBaseURL: true) else {
226-
return completion(.failure(RegistryError.invalidURL(registry.url)))
225+
return .failure(RegistryError.invalidURL(registry.url))
227226
}
228227
components.appendPathComponents("\(package.scope)", "\(package.name)")
229228
guard let url = components.url else {
230-
return completion(.failure(RegistryError.invalidURL(registry.url)))
229+
return .failure(RegistryError.invalidURL(registry.url))
231230
}
232231

233-
let request = LegacyHTTPClient.Request(
234-
method: .get,
235-
url: url,
236-
headers: [
237-
"Accept": self.acceptHeader(mediaType: .json),
238-
],
239-
options: self.defaultRequestOptions(timeout: timeout, callbackQueue: callbackQueue)
240-
)
232+
// If the responses are paginated then iterate until we've exasuasted all the pages and have a full versions list.
233+
func iterateResponses(url: URL, existingMetadata: PackageMetadata) async -> Result<PackageMetadata, Error> {
234+
let response = await self._getIndividualPackageMetadata(
235+
url: url,
236+
registry: registry,
237+
package: package,
238+
timeout: timeout,
239+
observabilityScope: observabilityScope,
240+
callbackQueue: callbackQueue
241+
)
241242

242-
let start = DispatchTime.now()
243-
observabilityScope.emit(info: "retrieving \(package) metadata from \(request.url)")
244-
self.httpClient.execute(request, observabilityScope: observabilityScope, progress: nil) { result in
245-
completion(
246-
result.tryMap { response in
247-
observabilityScope
248-
.emit(
249-
debug: "server response for \(request.url): \(response.statusCode) in \(start.distance(to: .now()).descriptionInSeconds)"
250-
)
251-
switch response.statusCode {
252-
case 200:
253-
let packageMetadata = try response.parseJSON(
254-
Serialization.PackageMetadata.self,
255-
decoder: self.jsonDecoder
243+
if case .success(let metadata) = response {
244+
let mergedMetadata = PackageMetadata(
245+
registry: registry,
246+
versions: existingMetadata.versions + metadata.versions,
247+
alternateLocations: existingMetadata.alternateLocations.count > 0
248+
? existingMetadata.alternateLocations
249+
: metadata.alternateLocations,
250+
nextPage: metadata.nextPage
251+
)
252+
if let nextPage = mergedMetadata.nextPage?.url {
253+
return await iterateResponses(url: nextPage, existingMetadata: mergedMetadata)
254+
} else {
255+
return .success(
256+
PackageMetadata(
257+
registry: registry,
258+
versions: mergedMetadata.versions.sorted(by: >),
259+
alternateLocations: mergedMetadata.alternateLocations,
260+
nextPage: mergedMetadata.nextPage
256261
)
262+
)
263+
}
264+
}
265+
return response
266+
}
257267

258-
let versions = packageMetadata.releases.filter { $0.value.problem == nil }
259-
.compactMap { Version($0.key) }
260-
.sorted(by: >)
268+
return await iterateResponses(url: url, existingMetadata: PackageMetadata(registry: registry, versions: [], alternateLocations: [], nextPage: nil))
269+
}
261270

262-
let alternateLocations = try response.headers.parseAlternativeLocationLinks()
271+
private func _getIndividualPackageMetadata(
272+
url: URL,
273+
registry: Registry,
274+
package: PackageIdentity.RegistryIdentity,
275+
timeout: DispatchTimeInterval?,
276+
observabilityScope: ObservabilityScope,
277+
callbackQueue: DispatchQueue
278+
) async -> Result<PackageMetadata, Error> {
279+
do {
280+
let start = DispatchTime.now()
281+
observabilityScope.emit(info: "retrieving \(package) metadata from \(url)")
282+
let response = try await self.httpClient.get(
283+
url,
284+
headers: [
285+
"Accept": self.acceptHeader(mediaType: .json),
286+
],
287+
options: self.defaultRequestOptions(timeout: timeout, callbackQueue: callbackQueue),
288+
observabilityScope: observabilityScope
289+
)
263290

264-
return PackageMetadata(
265-
registry: registry,
266-
versions: versions,
267-
alternateLocations: alternateLocations?.map(\.url)
268-
)
269-
case 404:
270-
throw RegistryError.packageNotFound
271-
default:
272-
throw self.unexpectedStatusError(response, expectedStatus: [200, 404])
273-
}
274-
}.mapError {
275-
RegistryError.failedRetrievingReleases(registry: registry, package: package.underlying, error: $0)
291+
observabilityScope
292+
.emit(
293+
debug: "server response for \(url): \(response.statusCode) in \(start.distance(to: .now()).descriptionInSeconds)"
294+
)
295+
296+
switch response.statusCode {
297+
case 200:
298+
let packageMetadata = try response.parseJSON(
299+
Serialization.PackageMetadata.self,
300+
decoder: self.jsonDecoder
301+
)
302+
303+
let versions = packageMetadata.releases.filter { $0.value.problem == nil }
304+
.compactMap { Version($0.key) }
305+
306+
let alternateLocations = response.headers.parseAlternativeLocationLinks()
307+
let paginationLinks = response.headers.parsePagniationLinks()
308+
309+
return .success(PackageMetadata(
310+
registry: registry,
311+
versions: versions,
312+
alternateLocations: alternateLocations.map(\.url),
313+
nextPage: paginationLinks.first { $0.kind == .next }?.url
314+
))
315+
case 404:
316+
return .failure(RegistryError.failedRetrievingReleases(registry: registry, package: package.underlying, error: RegistryError.packageNotFound))
317+
default:
318+
return .failure(RegistryError.failedRetrievingReleases(registry: registry, package: package.underlying, error: self.unexpectedStatusError(response, expectedStatus: [200, 404])))
276319
}
277-
)
320+
} catch {
321+
return .failure(RegistryError.failedRetrievingReleases(registry: registry, package: package.underlying, error: error))
278322
}
279323
}
280324

@@ -2071,7 +2115,8 @@ extension RegistryClient {
20712115
public struct PackageMetadata {
20722116
public let registry: Registry
20732117
public let versions: [Version]
2074-
public let alternateLocations: [SourceControlURL]?
2118+
public let alternateLocations: [SourceControlURL]
2119+
public let nextPage: SourceControlURL?
20752120
}
20762121

20772122
public struct PackageVersionMetadata: Sendable {
@@ -2142,6 +2187,17 @@ extension RegistryClient {
21422187
case alternate
21432188
}
21442189
}
2190+
2191+
fileprivate struct NextLocationLink {
2192+
let url: SourceControlURL
2193+
let kind: Kind
2194+
2195+
enum Kind: String {
2196+
// Currently we only care about `next` for pagination, but there are several other values:
2197+
// https://github.com/swiftlang/swift-package-manager/blob/0340bb12a56f9696b3966ad82c2aee1594135377/Documentation/PackageRegistry/Registry.md?plain=1#L403-L411
2198+
case next
2199+
}
2200+
}
21452201
}
21462202

21472203
extension RegistryClient {
@@ -2263,20 +2319,16 @@ extension HTTPClientResponse {
22632319
}
22642320

22652321
extension HTTPClientHeaders {
2266-
/*
2267-
<https://github.com/mona/LinkedList>; rel="canonical",
2268-
<ssh://[email protected]:mona/LinkedList.git>; rel="alternate",
2269-
*/
2270-
fileprivate func parseAlternativeLocationLinks() throws -> [RegistryClient.AlternativeLocationLink]? {
2271-
try self.get("Link").map { header -> [RegistryClient.AlternativeLocationLink] in
2322+
fileprivate func parseLink<T>(_ factory: (String) throws -> T?) rethrows -> [T] {
2323+
return try self.get("Link").map { header -> [T] in
22722324
let linkLines = header.split(separator: ",").map(String.init).map { $0.spm_chuzzle() ?? $0 }
22732325
return try linkLines.compactMap { linkLine in
2274-
try parseAlternativeLocationLine(linkLine)
2326+
try factory(linkLine)
22752327
}
22762328
}.flatMap { $0 }
22772329
}
22782330

2279-
private func parseAlternativeLocationLine(_ value: String) throws -> RegistryClient.AlternativeLocationLink? {
2331+
fileprivate func parseLocationLine<T>(_ value: String, _ factory: (String, String) -> T?) -> T? {
22802332
let fields = value.split(separator: ";")
22812333
.map(String.init)
22822334
.map { $0.spm_chuzzle() ?? $0 }
@@ -2290,16 +2342,60 @@ extension HTTPClientHeaders {
22902342
return nil
22912343
}
22922344

2293-
guard let rel = fields.first(where: { $0.hasPrefix("rel=") }).flatMap({ parseLinkFieldValue($0) }),
2294-
let kind = RegistryClient.AlternativeLocationLink.Kind(rawValue: rel)
2345+
guard let rel = fields.first(where: { $0.hasPrefix("rel=") }).flatMap({ parseLinkFieldValue($0) })
22952346
else {
22962347
return nil
22972348
}
22982349

2299-
return RegistryClient.AlternativeLocationLink(
2300-
url: SourceControlURL(link),
2301-
kind: kind
2302-
)
2350+
return factory(link, rel)
2351+
}
2352+
}
2353+
2354+
extension HTTPClientHeaders {
2355+
/*
2356+
https://github.com/swiftlang/swift-package-manager/blob/0340bb12a56f9696b3966ad82c2aee1594135377/Documentation/PackageRegistry/Registry.md?plain=1#L395C1-L401C39
2357+
<https://github.com/mona/LinkedList>; rel="canonical",
2358+
<ssh://[email protected]:mona/LinkedList.git>; rel="alternate",
2359+
*/
2360+
fileprivate func parseAlternativeLocationLinks() -> [RegistryClient.AlternativeLocationLink] {
2361+
self.parseLink(self.parseAlternativeLocationLine(_:))
2362+
}
2363+
2364+
private func parseAlternativeLocationLine(_ value: String) -> RegistryClient.AlternativeLocationLink? {
2365+
return parseLocationLine(value) { link, rel in
2366+
guard let kind = RegistryClient.AlternativeLocationLink.Kind(rawValue: rel) else {
2367+
return nil
2368+
}
2369+
2370+
return RegistryClient.AlternativeLocationLink(
2371+
url: SourceControlURL(link),
2372+
kind: kind
2373+
)
2374+
}
2375+
}
2376+
}
2377+
2378+
extension HTTPClientHeaders {
2379+
/*
2380+
https://github.com/swiftlang/swift-package-manager/blob/0340bb12a56f9696b3966ad82c2aee1594135377/Documentation/PackageRegistry/Registry.md?plain=1#L403-L411
2381+
<https://github.com/mona/LinkedList?page=2>; rel="next",
2382+
<ssh://[email protected]:mona/LinkedList.git?page=40>; rel="last",
2383+
*/
2384+
fileprivate func parsePagniationLinks() -> [RegistryClient.NextLocationLink] {
2385+
self.parseLink(self.parsePaginationLine(_:))
2386+
}
2387+
2388+
private func parsePaginationLine(_ value: String) -> RegistryClient.NextLocationLink? {
2389+
return parseLocationLine(value) { link, rel in
2390+
guard let kind = RegistryClient.NextLocationLink.Kind(rawValue: rel) else {
2391+
return nil
2392+
}
2393+
2394+
return RegistryClient.NextLocationLink(
2395+
url: SourceControlURL(link),
2396+
kind: kind
2397+
)
2398+
}
23032399
}
23042400
}
23052401

@@ -2308,12 +2404,7 @@ extension HTTPClientHeaders {
23082404
<http://packages.example.com/mona/LinkedList/1.1.1/Package.swift?swift-version=4>; rel="alternate"; filename="[email protected]"; swift-tools-version="4.0"
23092405
*/
23102406
fileprivate func parseManifestLinks() throws -> [RegistryClient.ManifestLink] {
2311-
try self.get("Link").map { header -> [RegistryClient.ManifestLink] in
2312-
let linkLines = header.split(separator: ",").map(String.init).map { $0.spm_chuzzle() ?? $0 }
2313-
return try linkLines.compactMap { linkLine in
2314-
try parseManifestLinkLine(linkLine)
2315-
}
2316-
}.flatMap { $0 }
2407+
try self.parseLink(self.parseManifestLinkLine(_:))
23172408
}
23182409

23192410
private func parseManifestLinkLine(_ value: String) throws -> RegistryClient.ManifestLink? {

Tests/PackageRegistryTests/RegistryClientTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ final class RegistryClientTests: XCTestCase {
8888
let registryClient = makeRegistryClient(configuration: configuration, httpClient: httpClient)
8989
let metadata = try await registryClient.getPackageMetadata(package: identity)
9090
XCTAssertEqual(metadata.versions, ["1.1.1", "1.0.0"])
91-
XCTAssertEqual(metadata.alternateLocations!, [
91+
XCTAssertEqual(metadata.alternateLocations, [
9292
SourceControlURL("https://github.com/mona/LinkedList"),
9393
SourceControlURL("ssh://[email protected]:mona/LinkedList.git"),
9494
SourceControlURL("[email protected]:mona/LinkedList.git"),

0 commit comments

Comments
 (0)