Skip to content

Commit f106efd

Browse files
finestructurefinestructure
authored
Merge pull request #3386 from SwiftPackageIndex/issue-3375
Return svg+xml content type for documentation SVG images
2 parents be7c143 + 8b44af9 commit f106efd

File tree

4 files changed

+70
-14
lines changed

4 files changed

+70
-14
lines changed

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,7 @@ enum PackageController {
5858
}
5959

6060
static func documentation(req: Request, route: DocRoute) async throws -> Response {
61-
let res: ClientResponse
62-
do {
63-
res = try await awsResponse(client: req.client, route: route)
64-
} catch {
65-
print(error)
66-
throw error
67-
}
61+
let res = try await awsResponse(client: req.client, route: route)
6862

6963
switch route.fragment {
7064
case .documentation, .tutorials:
@@ -78,7 +72,7 @@ enum PackageController {
7872
documentationMetadata: documentationMetadata
7973
)
8074

81-
case .css, .data, .faviconIco, .faviconSvg, .images, .img, .index, .js, .linkablePaths, .themeSettings:
75+
case .css, .data, .faviconIco, .faviconSvg, .images, .img, .index, .js, .linkablePaths, .themeSettings, .svgImages, .svgImg:
8276
return try await res.encodeResponse(
8377
status: .ok,
8478
headers: req.headers
@@ -452,8 +446,8 @@ extension PackageController {
452446
let path = route.path
453447

454448
switch route.fragment {
455-
case .css, .data, .documentation, .images, .img, .index, .js, .tutorials:
456-
return URI(string: "\(baseURL)/\(route.fragment)/\(path)")
449+
case .css, .data, .documentation, .images, .img, .index, .js, .tutorials, .svgImages, .svgImg:
450+
return URI(string: "\(baseURL)/\(route.fragment.urlFragment)/\(path)")
457451
case .faviconIco, .faviconSvg, .themeSettings:
458452
return path.isEmpty
459453
? URI(string: "\(baseURL)/\(route.fragment)")

‎Sources/App/Core/DocRoute.swift‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ struct DocRoute: Equatable {
3535
case linkablePaths = "linkable-paths.json"
3636
case themeSettings = "theme-settings.json"
3737
case tutorials
38+
case svgImages
39+
case svgImg
3840

3941
var contentType: String {
4042
switch self {
@@ -48,18 +50,31 @@ struct DocRoute: Equatable {
4850
return "text/html; charset=utf-8"
4951
case .js:
5052
return "application/javascript"
53+
case .svgImages, .svgImg:
54+
return "image/svg+xml"
5155
}
5256
}
5357

5458
var requiresArchive: Bool {
5559
switch self {
56-
case .css, .data, .faviconIco, .faviconSvg, .images, .img, .index, .js, .linkablePaths, .themeSettings, .tutorials:
60+
case .css, .data, .faviconIco, .faviconSvg, .images, .img, .index, .js, .linkablePaths, .themeSettings, .tutorials, .svgImages, .svgImg:
5761
return false
5862
case .documentation:
5963
return true
6064
}
6165
}
6266

67+
var urlFragment: String {
68+
switch self {
69+
case .css, .data, .documentation, .faviconIco, .faviconSvg, .images, .img, .index, .js, .linkablePaths, .themeSettings, .tutorials:
70+
return rawValue
71+
case .svgImages:
72+
return "images"
73+
case .svgImg:
74+
return "img"
75+
}
76+
}
77+
6378
var description: String { rawValue }
6479
}
6580
}

‎Sources/App/routes+documentation.swift‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,13 @@ func docRoutes(_ app: Application) throws {
5959
return try await PackageController.documentation(req: $0, route: route)
6060
}.excludeFromOpenAPI()
6161
app.get(":owner", ":repository", ":reference", "images", "**") {
62-
let route = try await $0.getDocRoute(fragment: .images)
62+
let fragment: DocRoute.Fragment = $0.parameters.hasSuffix(".svg", caseInsensitive: true) ? .svgImages : .images
63+
let route = try await $0.getDocRoute(fragment: fragment)
6364
return try await PackageController.documentation(req: $0, route: route)
6465
}.excludeFromOpenAPI()
6566
app.get(":owner", ":repository", ":reference", "img", "**") {
66-
let route = try await $0.getDocRoute(fragment: .img)
67+
let fragment: DocRoute.Fragment = $0.parameters.hasSuffix(".svg", caseInsensitive: true) ? .svgImg : .img
68+
let route = try await $0.getDocRoute(fragment: fragment)
6769
return try await PackageController.documentation(req: $0, route: route)
6870
}.excludeFromOpenAPI()
6971
app.get(":owner", ":repository", ":reference", "index", "**") {
@@ -109,10 +111,18 @@ private extension Parameters {
109111
// AND THE FIX
110112
// https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/pull/3039
111113
return ([archive].compactMap { $0 } + getCatchall()).map { $0.lowercased() }
112-
case .css, .faviconIco, .faviconSvg, .images, .img, .index, .js, .linkablePaths, .themeSettings:
114+
case .css, .faviconIco, .faviconSvg, .images, .img, .index, .js, .linkablePaths, .themeSettings, .svgImages, .svgImg:
113115
return getCatchall()
114116
}
115117
}
118+
119+
func hasSuffix(_ suffix: String, caseInsensitive: Bool) -> Bool {
120+
if caseInsensitive {
121+
return getCatchall().last?.lowercased().hasSuffix(suffix.lowercased()) ?? false
122+
} else {
123+
return getCatchall().last?.hasSuffix(suffix) ?? false
124+
}
125+
}
116126
}
117127

118128

‎Tests/AppTests/PackageController+routesTests.swift‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,22 @@ class PackageController_routesTests: SnapshotTestCase {
343343
try PackageController.awsDocumentationURL(route: .init(owner: "Foo", repository: "Bar", docVersion: .reference("1.2.3"), fragment: .data, pathElements: ["path"])).string,
344344
"http://docs-bucket.s3-website.us-east-2.amazonaws.com/foo/bar/1.2.3/data/path"
345345
)
346+
XCTAssertEqual(
347+
try PackageController.awsDocumentationURL(route: .init(owner: "Foo", repository: "Bar", docVersion: .reference("1.2.3"), fragment: .images, pathElements: ["path"])).string,
348+
"http://docs-bucket.s3-website.us-east-2.amazonaws.com/foo/bar/1.2.3/images/path"
349+
)
350+
XCTAssertEqual(
351+
try PackageController.awsDocumentationURL(route: .init(owner: "Foo", repository: "Bar", docVersion: .reference("1.2.3"), fragment: .img, pathElements: ["path"])).string,
352+
"http://docs-bucket.s3-website.us-east-2.amazonaws.com/foo/bar/1.2.3/img/path"
353+
)
354+
XCTAssertEqual(
355+
try PackageController.awsDocumentationURL(route: .init(owner: "Foo", repository: "Bar", docVersion: .reference("1.2.3"), fragment: .svgImages, pathElements: ["path"])).string,
356+
"http://docs-bucket.s3-website.us-east-2.amazonaws.com/foo/bar/1.2.3/images/path"
357+
)
358+
XCTAssertEqual(
359+
try PackageController.awsDocumentationURL(route: .init(owner: "Foo", repository: "Bar", docVersion: .reference("1.2.3"), fragment: .svgImg, pathElements: ["path"])).string,
360+
"http://docs-bucket.s3-website.us-east-2.amazonaws.com/foo/bar/1.2.3/img/path"
361+
)
346362
XCTAssertEqual(
347363
try PackageController.awsDocumentationURL(route: .init(owner: "Foo", repository: "Bar", docVersion: .reference("1.2.3"), fragment: .js, pathElements: ["path"])).string,
348364
"http://docs-bucket.s3-website.us-east-2.amazonaws.com/foo/bar/1.2.3/js/path"
@@ -427,6 +443,27 @@ class PackageController_routesTests: SnapshotTestCase {
427443
"/owner/repo/canonical-ref/documentation/archive/symbol:$-%")
428444
}
429445

446+
func test_documentation_routes_contentType() async throws {
447+
try await app.test(.GET, "/owner/package/main/images/foo/bar.jpeg") { res async in
448+
XCTAssertEqual(res.headers.contentType, .init(type: "application", subType: "octet-stream"))
449+
}
450+
try await app.test(.GET, "/owner/package/main/images/foo/bar.svg") { res async in
451+
XCTAssertEqual(res.headers.contentType, .init(type: "image", subType: "svg+xml"))
452+
}
453+
try await app.test(.GET, "/owner/package/main/images/foo/bar.SVG") { res async in
454+
XCTAssertEqual(res.headers.contentType, .init(type: "image", subType: "svg+xml"))
455+
}
456+
try await app.test(.GET, "/owner/package/main/img/foo/bar.jpeg") { res async in
457+
XCTAssertEqual(res.headers.contentType, .init(type: "application", subType: "octet-stream"))
458+
}
459+
try await app.test(.GET, "/owner/package/main/img/foo/bar.svg") { res async in
460+
XCTAssertEqual(res.headers.contentType, .init(type: "image", subType: "svg+xml"))
461+
}
462+
try await app.test(.GET, "/owner/package/main/img/foo/bar.SVG") { res async in
463+
XCTAssertEqual(res.headers.contentType, .init(type: "image", subType: "svg+xml"))
464+
}
465+
}
466+
430467
func test_documentation_routes_redirect() async throws {
431468
// Test the redirect documentation routes without any reference:
432469
// /owner/package/documentation + various path elements

0 commit comments

Comments
 (0)