Skip to content

Commit 61d8415

Browse files
daveverwerdaveverwer
authored
Merge pull request #3047 from SwiftPackageIndex/documentation-target-typed-reference
Give `DocumentationTarget` references some type safety
2 parents 034fd9b + 6d85dbf commit 61d8415

15 files changed

+104
-74
lines changed

‎Sources/App/Controllers/PackageController+routes.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ extension PackageController {
465465
static func canonicalDocumentationUrl(from url: String,
466466
owner: String,
467467
repository: String,
468-
docVersion: DocRoute.DocVersion,
468+
docVersion: DocVersion,
469469
toTarget target: DocumentationTarget) -> String? {
470470
// It's important to use `docVersion.reference` here to make sure we match with true reference urls and not ~
471471
let urlPrefix = "/\(owner)/\(repository)/\(docVersion.reference.pathEncoded)/"

‎Sources/App/Core/DocRoute.swift‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,29 +22,6 @@ struct DocRoute: Equatable {
2222

2323
var contentType: String { fragment.contentType }
2424

25-
enum DocVersion: CustomStringConvertible, Equatable {
26-
case current(referencing: String)
27-
case reference(String)
28-
29-
var description: String {
30-
switch self {
31-
case .current:
32-
return "~"
33-
case .reference(let string):
34-
return string
35-
}
36-
}
37-
38-
var reference: String {
39-
switch self {
40-
case .current(let reference):
41-
return reference
42-
case .reference(let reference):
43-
return reference
44-
}
45-
}
46-
}
47-
4825
enum Fragment: String, CustomStringConvertible {
4926
case css
5027
case data

‎Sources/App/Core/DocVersion.swift‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
16+
enum DocVersion: Codable, Equatable {
17+
case current(referencing: String? = nil)
18+
case reference(String)
19+
20+
var reference: String {
21+
switch self {
22+
case .current(.none):
23+
return String.current
24+
case .current(.some(let reference)):
25+
return reference
26+
case .reference(let reference):
27+
return reference
28+
}
29+
}
30+
31+
var pathEncoded: String {
32+
switch self {
33+
case .current: String.current
34+
case .reference( let reference): reference.pathEncoded
35+
}
36+
}
37+
}
38+
39+
40+
extension DocVersion: CustomStringConvertible {
41+
var description: String {
42+
switch self {
43+
case .current:
44+
return String.current
45+
case .reference(let string):
46+
return string
47+
}
48+
}
49+
}

‎Sources/App/Core/DocumentationTarget.swift‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import Fluent
1717

1818
enum DocumentationTarget: Equatable, Codable {
1919
case external(url: String)
20-
case `internal`(reference: String, archive: String)
20+
case `internal`(docVersion: DocVersion, archive: String)
2121

2222
/// Fetch DocumentationTarget for a given package.
2323
/// - Parameters:
@@ -58,7 +58,7 @@ enum DocumentationTarget: Equatable, Codable {
5858
/// - repository: Repository name
5959
/// - reference: Version reference
6060
/// - Returns: DocumentationTarget or nil
61-
static func query(on database: Database, owner: String, repository: String, reference: Reference) async throws -> Self? {
61+
static func query(on database: Database, owner: String, repository: String, docVersion: DocVersion) async throws -> Self? {
6262
let archive = try await Joined3<Version, Package, Repository>
6363
.query(on: database,
6464
join: \Version.$package.$id == \Package.$id, method: .inner,
@@ -71,23 +71,23 @@ enum DocumentationTarget: Equatable, Codable {
7171
.all()
7272
// we need to filter client side to support pathEncoded reference equality
7373
// https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/2287
74-
.first(where: { $0.model.reference.pathEncoded == reference.pathEncoded })
74+
.first(where: { $0.model.reference.pathEncoded == docVersion.pathEncoded })
7575
.flatMap { $0.model.docArchives?.first?.name }
7676

7777
return archive.map {
78-
.internal(reference: "\(reference)", archive: $0)
78+
.internal(docVersion: docVersion, archive: $0)
7979
}
8080
}
8181
}
8282

8383

8484
extension DocumentationTarget {
85-
var `internal`: (reference: String, archive: String)? {
85+
var `internal`: (docVersion: DocVersion, archive: String)? {
8686
switch self {
8787
case .external:
8888
return nil
89-
case .internal(let reference, let archive):
90-
return (reference, archive)
89+
case .internal(let docVersion, let archive):
90+
return (docVersion, archive)
9191
}
9292
}
9393
}

‎Sources/App/Core/Extensions/Array+Version.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,19 @@ extension [Version] {
2828
// Ideal case is that we have a stable release documentation.
2929
if let version = releaseVersion,
3030
let archive = version.docArchives?.first?.name {
31-
return .internal(reference: "\(version.reference)", archive: archive)
31+
return .internal(docVersion: .reference("\(version.reference)"), archive: archive)
3232
}
3333

3434
// Then a pre-release is second best.
3535
if let version = preReleaseVersion,
3636
let archive = version.docArchives?.first?.name {
37-
return .internal(reference: "\(version.reference)", archive: archive)
37+
return .internal(docVersion: .reference("\(version.reference)"), archive: archive)
3838
}
3939

4040
// Finally, fallback to the default branch documentation.
4141
if let version = defaultBranchVersion,
4242
let archive = version.docArchives?.first?.name {
43-
return .internal(reference: "\(version.reference)", archive: archive)
43+
return .internal(docVersion: .reference("\(version.reference)"), archive: archive)
4444
}
4545

4646
// There is no default dodcumentation.

‎Sources/App/Views/DocumentationPageProcessor.swift‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ struct DocumentationPageProcessor {
2222
let repositoryOwnerName: String
2323
let repositoryName: String
2424
let packageName: String
25-
let docVersion: DocRoute.DocVersion
25+
let docVersion: DocVersion
2626
let referenceLatest: Version.Kind?
2727
let referenceKind: Version.Kind
2828
let canonicalUrl: String?
@@ -49,7 +49,7 @@ struct DocumentationPageProcessor {
4949
repositoryOwnerName: String,
5050
repositoryName: String,
5151
packageName: String,
52-
docVersion: DocRoute.DocVersion,
52+
docVersion: DocVersion,
5353
referenceLatest: Version.Kind?,
5454
referenceKind: Version.Kind,
5555
canonicalUrl: String?,
@@ -134,7 +134,7 @@ struct DocumentationPageProcessor {
134134
SiteURL.relativeURL(
135135
owner: repositoryOwner,
136136
repository: repositoryName,
137-
documentation: .internal(reference: version.reference,
137+
documentation: .internal(docVersion: .reference(version.reference),
138138
archive: currentArchive.name),
139139
fragment: .documentation
140140
)
@@ -171,7 +171,7 @@ struct DocumentationPageProcessor {
171171
SiteURL.relativeURL(
172172
owner: repositoryOwner,
173173
repository: repositoryName,
174-
documentation: .internal(reference: docVersion.reference, archive: archive.name),
174+
documentation: .internal(docVersion: docVersion, archive: archive.name),
175175
fragment: .documentation
176176
)
177177
),
@@ -210,7 +210,7 @@ struct DocumentationPageProcessor {
210210
SiteURL.relativeURL(
211211
owner: repositoryOwner,
212212
repository: repositoryName,
213-
documentation: .internal(reference: latestStable.reference,
213+
documentation: .internal(docVersion: .reference(latestStable.reference),
214214
archive: docArchive.name),
215215
fragment: .documentation
216216
)
@@ -308,13 +308,13 @@ struct DocumentationPageProcessor {
308308
)
309309
}
310310

311-
static func rewriteBaseUrls(document: SwiftSoup.Document, owner: String, repository: String, docVersion: DocRoute.DocVersion) throws {
311+
static func rewriteBaseUrls(document: SwiftSoup.Document, owner: String, repository: String, docVersion: DocVersion) throws {
312312
try rewriteScriptBaseUrl(document: document, owner: owner, repository: repository, docVersion: docVersion)
313313
try rewriteAttribute("href", document: document, owner: owner, repository: repository, docVersion: docVersion)
314314
try rewriteAttribute("src", document: document, owner: owner, repository: repository, docVersion: docVersion)
315315
}
316316

317-
static func rewriteScriptBaseUrl(document: SwiftSoup.Document, owner: String, repository: String, docVersion: DocRoute.DocVersion) throws {
317+
static func rewriteScriptBaseUrl(document: SwiftSoup.Document, owner: String, repository: String, docVersion: DocVersion) throws {
318318
// Possible rewrite strategies
319319
// / -> /a/b/1.2.3 (toReference)
320320
// / -> /a/b/~ (current)
@@ -328,11 +328,13 @@ struct DocumentationPageProcessor {
328328
let path = "/\(owner)/\(repository)/\(String.current)/".lowercased()
329329
try e.html(#"var baseUrl = "\#(path)""#)
330330
}
331-
let fullyQualifiedPrefix = "/\(owner)/\(repository)/\(reference)".lowercased()
332-
if value == #"var baseUrl = "\#(fullyQualifiedPrefix)/""# {
333-
// /a/b/1.2.3 -> /a/b/~ (current)
334-
let path = "/\(owner)/\(repository)/\(String.current)/".lowercased()
335-
try e.html(#"var baseUrl = "\#(path)""#)
331+
if let reference {
332+
let fullyQualifiedPrefix = "/\(owner)/\(repository)/\(reference)".lowercased()
333+
if value == #"var baseUrl = "\#(fullyQualifiedPrefix)/""# {
334+
// /a/b/1.2.3 -> /a/b/~ (current)
335+
let path = "/\(owner)/\(repository)/\(String.current)/".lowercased()
336+
try e.html(#"var baseUrl = "\#(path)""#)
337+
}
336338
}
337339
}
338340
case .reference(let reference):
@@ -347,7 +349,7 @@ struct DocumentationPageProcessor {
347349
}
348350
}
349351

350-
static func rewriteAttribute(_ attribute: String, document: SwiftSoup.Document, owner: String, repository: String, docVersion: DocRoute.DocVersion) throws {
352+
static func rewriteAttribute(_ attribute: String, document: SwiftSoup.Document, owner: String, repository: String, docVersion: DocVersion) throws {
351353
// Possible rewrite strategies
352354
// / -> /a/b/1.2.3 (toReference)
353355
// / -> /a/b/~ (current)
@@ -360,7 +362,7 @@ struct DocumentationPageProcessor {
360362
// no /{owner}/{repo}/ prefix -> it's a dynamic base url resource, i.e. a "/" resource
361363
// / -> /a/b/~ (current)
362364
try e.attr(attribute, "/\(owner)/\(repository)/\(String.current)\(value)".lowercased())
363-
} else {
365+
} else if let reference {
364366
let fullyQualifiedPrefix = "/\(owner)/\(repository)/\(reference)".lowercased()
365367
if value.lowercased().hasPrefix(fullyQualifiedPrefix) {
366368
// matches expected fully qualified resource path

‎Sources/App/Views/PackageController/MaintainerInfo/MaintainerInfoIndex+View.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,15 @@ enum MaintainerInfoIndex {
5757
static func spiManifestCommonUseCasesDocLink(_ anchor: Anchor) -> String {
5858
SiteURL.relativeURL(owner: "SwiftPackageIndex",
5959
repository: "SPIManifest",
60-
documentation: .internal(reference: .current, archive: "spimanifest"),
60+
documentation: .internal(docVersion: .current(), archive: "spimanifest"),
6161
fragment: .documentation,
6262
path: "spimanifest/commonusecases#\(anchor)")
6363
}
6464

6565
static func spiManifestDocLink() -> String {
6666
SiteURL.relativeURL(owner: "SwiftPackageIndex",
6767
repository: "SPIManifest",
68-
documentation: .internal(reference: .current, archive: "spimanifest"),
68+
documentation: .internal(docVersion: .current(), archive: "spimanifest"),
6969
fragment: .documentation)
7070
}
7171

‎Sources/App/routes+documentation.swift‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ extension Request {
139139
if ref == .current {
140140
target = try await DocumentationTarget.query(on: db, owner: owner, repository: repository)
141141
} else {
142-
target = try await DocumentationTarget.query(on: db, owner: owner, repository: repository, reference: .init(ref))
142+
target = try await DocumentationTarget.query(on: db, owner: owner, repository: repository,
143+
docVersion: .reference(ref))
143144
}
144145

145146
case .none:
@@ -159,16 +160,17 @@ extension Request {
159160
if fragment.requiresArchive && archive == nil { throw Abort(.badRequest) }
160161
let pathElements = parameters.pathElements(for: fragment, archive: archive)
161162

162-
let docVersion = try await { () -> DocRoute.DocVersion in
163+
let docVersion = try await { () -> DocVersion in
163164
if reference == String.current {
164165
if let ref = Current.currentReferenceCache()?[owner: owner, repository: repository] {
165166
return .current(referencing: ref)
166167
}
167-
guard let params = try await DocumentationTarget.query(on: db, owner: owner, repository: repository)?.internal else {
168-
throw Abort(.notFound)
169-
}
170-
Current.currentReferenceCache()?[owner: owner, repository: repository] = params.reference
171-
return .current(referencing: params.reference)
168+
169+
guard let params = try await DocumentationTarget.query(on: db, owner: owner, repository: repository)?.internal
170+
else { throw Abort(.notFound) }
171+
172+
Current.currentReferenceCache()?[owner: owner, repository: repository] = "\(params.docVersion)"
173+
return .current(referencing: "\(params.docVersion)")
172174
} else {
173175
return .reference(reference)
174176
}

‎Tests/AppTests/API+PackageController+GetRoute+ModelTests.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class API_PackageController_GetRoute_ModelTests: SnapshotTestCase {
8686
weightedKeywords: []))
8787

8888
// validate
89-
XCTAssertEqual(model.documentationTarget, .internal(reference: "main", archive: "archive1"))
89+
XCTAssertEqual(model.documentationTarget, .internal(docVersion: .reference("main"), archive: "archive1"))
9090
}
9191

9292
func test_init_external_documentation() async throws {

‎Tests/AppTests/ArrayExtensionTests.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ class ArrayExtensionTests: XCTestCase {
167167
Version(id: .id0, latest: .release, reference: .tag(1, 2, 3),
168168
docArchives: [.init(name: "foo", title: "Foo")]),
169169
].canonicalDocumentationTarget(),
170-
.internal(reference: "1.2.3", archive: "foo")
170+
.internal(docVersion: .reference("1.2.3"), archive: "foo")
171171
)
172172

173173
// Test default branch fallback
@@ -177,7 +177,7 @@ class ArrayExtensionTests: XCTestCase {
177177
Version(id: .id0, latest: .defaultBranch, reference: .branch("main"),
178178
docArchives: [.init(name: "foo", title: "Foo")]),
179179
].canonicalDocumentationTarget(),
180-
.internal(reference: "main", archive: "foo")
180+
.internal(docVersion: .reference("main"), archive: "foo")
181181
)
182182

183183
// No default branch version available

0 commit comments

Comments
 (0)