Skip to content

Commit

Permalink
Merge pull request #2705 from SwiftPackageIndex/sitemaps-tweaks
Browse files Browse the repository at this point in the history
Sitemaps tweaks
  • Loading branch information
finestructure authored Nov 9, 2023
2 parents d0a5b2f + 0b1800a commit 5838f9e
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 101 deletions.
8 changes: 4 additions & 4 deletions Sources/App/Controllers/PackageController+routes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,10 @@ enum PackageController {
[String]()
}

return try await SiteMapView.package(owner: packageResult.repository.owner,
repository: packageResult.repository.name,
lastActivityAt: packageResult.repository.lastActivityAt,
linkablePathUrls: urls)
return try await SiteMapController.package(owner: packageResult.repository.owner,
repository: packageResult.repository.name,
lastActivityAt: packageResult.repository.lastActivityAt,
linkablePathUrls: urls)
}

static func linkablePathUrls(client: Client, packageResult: PackageResult) async -> [String] {
Expand Down
82 changes: 80 additions & 2 deletions Sources/App/Controllers/SiteMapController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ import Plot

enum SiteMapController {

static var staticRoutes: [SiteURL] = [
.home,
.addAPackage,
.faq,
.supporters,
.buildMonitor,
.privacy
]

struct Package: Equatable, Decodable {
var owner: String
var repository: String
Expand All @@ -31,6 +40,9 @@ enum SiteMapController {
}
}

/// Generate a sitemap index page for a given request
/// - Parameter req: `Request`
/// - Returns: `SiteMapIndex`
static func index(req: Request) async throws -> SiteMapIndex {
guard let db = req.db as? SQLDatabase else {
fatalError("Database must be an SQLDatabase ('as? SQLDatabase' must succeed)")
Expand All @@ -46,10 +58,76 @@ enum SiteMapController {
.orderBy(Search.repoName)

let packages = try await query.all(decoding: Package.self)
return SiteMapView.index(packages: packages)
return index(packages: packages)
}

/// Generate a sitemap index page for a given list of packages
/// - Parameter packages: list of packages
/// - Returns: `SiteMapIndex`
static func index(packages: [SiteMapController.Package]) -> SiteMapIndex {
SiteMapIndex(
.sitemap(
.loc(SiteURL.siteMapStaticPages.absoluteURL()),
.lastmod(Current.date(), timeZone: .utc) // The home page updates every day.
),
.group(
packages.map { package -> Node<SiteMapIndex.SiteMapIndexContext> in
.sitemap(
.loc(SiteURL.package(.value(package.owner),
.value(package.repository),
.siteMap).absoluteURL()),
.unwrap(package.lastActivityAt, { .lastmod($0, timeZone: .utc) })
)
}
)
)
}

/// Generate a sitemap for a specific package
/// - Parameters:
/// - owner: package owner (author)
/// - repository: repository name
/// - lastActivityAt: last activity
/// - linkablePathUrls: list of linkable path urls
/// - Returns: `SiteMap`
static func package(owner: String?,
repository: String?,
lastActivityAt: Date?,
linkablePathUrls: [String]) async throws -> SiteMap {
guard let owner,
let repository
else {
// This should never happen, but we should return an empty
// sitemap instead of an incorrect one.
return SiteMap()
}

// See SwiftPackageIndex-Server#2485 for context on the performance implications of this
let lastmod: Node<SiteMap.URLContext> = lastActivityAt.map { .lastmod($0, timeZone: .utc) } ?? .empty

return SiteMap(
.url(
.loc(SiteURL.package(.value(owner),
.value(repository),
.none).absoluteURL()),
lastmod
),
.forEach(linkablePathUrls, { url in
.url(.loc(url), lastmod)
})
)
}

static func staticPages(req: Request) async throws -> SiteMap {
return SiteMapView.staticPages()
SiteMap(
.group(
staticRoutes.map { page -> Node<SiteMap.URLSetContext> in
.url(
.loc(page.absoluteURL())
)
}
)
)
}

}
87 changes: 0 additions & 87 deletions Sources/App/Views/SiteMap/SiteMapView.swift

This file was deleted.

43 changes: 43 additions & 0 deletions Tests/AppTests/PackageController+routesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,49 @@ class PackageController_routesTests: AppTestCase {
XCTAssertEqual(latestMajorRerefences, ["1.1.2", "2.1.1", "3.0.0"])
}

func test_siteMap_prod() async throws {
// Ensure sitemap routing is configured in prod
// Setup
Current.environment = { .production }
// We also need to set up a new app that's configured for production,
// because app.test is not affected by Current overrides.
let prodApp = try await setup(.production)
defer { prodApp.shutdown() }

// setup
let package = Package(url: URL(stringLiteral: "https://example.com/owner/repo0"))
try await package.save(on: app.db)
try await Repository(package: package, defaultBranch: "default",
lastCommitDate: Current.date(),
name: "Repo0", owner: "Owner").save(on: app.db)
try await Version(package: package, latest: .defaultBranch, packageName: "SomePackage",
reference: .branch("default")).save(on: app.db)

// MUT
try prodApp.test(.GET, "/owner/repo0/sitemap.xml") { res in
XCTAssertEqual(res.status, .ok)
}
}

func test_siteMap_dev() async throws {
// Ensure we don't serve sitemaps in dev
// app and Current.environment are configured for .development by default

// setup
let package = Package(url: URL(stringLiteral: "https://example.com/owner/repo0"))
try await package.save(on: app.db)
try await Repository(package: package, defaultBranch: "default",
lastCommitDate: Current.date(),
name: "Repo0", owner: "Owner").save(on: app.db)
try await Version(package: package, latest: .defaultBranch, packageName: "SomePackage",
reference: .branch("default")).save(on: app.db)

// MUT
try app.test(.GET, "/owner/repo0/sitemap.xml") { res in
XCTAssertEqual(res.status, .notFound)
}
}

func test_issue_2288() async throws {
// Ensures default branch updates don't introduce a "documentation gap"
// https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/2288
Expand Down
69 changes: 61 additions & 8 deletions Tests/AppTests/SitemapTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,33 @@ class SitemapTests: SnapshotTestCase {
as: .init(pathExtension: "xml", diffing: .lines))
}

func test_siteMapIndex_prod() async throws {
// Ensure sitemap routing is configured in prod
// Setup
Current.environment = { .production }
// We also need to set up a new app that's configured for production,
// because app.test is not affected by Current overrides.
let prodApp = try await setup(.production)
defer { prodApp.shutdown() }

// MUT
try prodApp.test(.GET, "/sitemap.xml") { res in
// Validation
XCTAssertEqual(res.status, .ok)
}
}

func test_siteMapIndex_dev() async throws {
// Ensure we don't serve sitemaps in dev
// app and Current.environment are configured for .development by default

// MUT
try app.test(.GET, "/sitemap.xml") { res in
// Validation
XCTAssertEqual(res.status, .notFound)
}
}

@MainActor
func test_siteMapStaticPages() async throws {
let req = Request(application: app, on: app.eventLoopGroup.next())
Expand All @@ -55,6 +82,32 @@ class SitemapTests: SnapshotTestCase {
as: .init(pathExtension: "xml", diffing: .lines))
}

func test_siteMapStaticPages_prod() async throws {
// Ensure sitemap routing is configured in prod
Current.environment = { .production }
// We also need to set up a new app that's configured for production,
// because app.test is not affected by Current overrides.
let prodApp = try await setup(.production)
defer { prodApp.shutdown() }

// MUT
try prodApp.test(.GET, "/sitemap-static-pages.xml") { res in
// Validation
XCTAssertEqual(res.status, .ok)
}
}

func test_siteMapStaticPages_dev() async throws {
// Ensure we don't serve sitemaps in dev
// app and Current.environment are configured for .development by default

// MUT
try app.test(.GET, "/sitemap-static-pages.xml") { res in
// Validation
XCTAssertEqual(res.status, .notFound)
}
}

func test_linkablePathUrls() async throws {
// setup
let package = Package(url: URL(stringLiteral: "https://example.com/owner/repo0"))
Expand Down Expand Up @@ -149,10 +202,10 @@ class SitemapTests: SnapshotTestCase {
.query(on: app.db, owner: "owner", repository: "repo0")

// MUT
let sitemap = try await SiteMapView.package(owner: packageResult.repository.owner,
repository: packageResult.repository.name,
lastActivityAt: packageResult.repository.lastActivityAt,
linkablePathUrls: [])
let sitemap = try await SiteMapController.package(owner: packageResult.repository.owner,
repository: packageResult.repository.name,
lastActivityAt: packageResult.repository.lastActivityAt,
linkablePathUrls: [])
let xml = sitemap.render(indentedBy: .spaces(2))

// Validation
Expand All @@ -178,10 +231,10 @@ class SitemapTests: SnapshotTestCase {
]

// MUT
let sitemap = try await SiteMapView.package(owner: packageResult.repository.owner,
repository: packageResult.repository.name,
lastActivityAt: packageResult.repository.lastActivityAt,
linkablePathUrls: linkablePathUrls)
let sitemap = try await SiteMapController.package(owner: packageResult.repository.owner,
repository: packageResult.repository.name,
lastActivityAt: packageResult.repository.lastActivityAt,
linkablePathUrls: linkablePathUrls)
let xml = sitemap.render(indentedBy: .spaces(2))

// Validation
Expand Down

0 comments on commit 5838f9e

Please sign in to comment.