diff --git a/Sources/App/Controllers/PackageController+routes.swift b/Sources/App/Controllers/PackageController+routes.swift index 2c3bad45a..94d3d6bcd 100644 --- a/Sources/App/Controllers/PackageController+routes.swift +++ b/Sources/App/Controllers/PackageController+routes.swift @@ -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] { diff --git a/Sources/App/Controllers/SiteMapController.swift b/Sources/App/Controllers/SiteMapController.swift index 88432c053..64ba1d89e 100644 --- a/Sources/App/Controllers/SiteMapController.swift +++ b/Sources/App/Controllers/SiteMapController.swift @@ -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 @@ -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)") @@ -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 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 = 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 in + .url( + .loc(page.absoluteURL()) + ) + } + ) + ) } + } diff --git a/Sources/App/Views/SiteMap/SiteMapView.swift b/Sources/App/Views/SiteMap/SiteMapView.swift deleted file mode 100644 index 05ba3ce7b..000000000 --- a/Sources/App/Views/SiteMap/SiteMapView.swift +++ /dev/null @@ -1,87 +0,0 @@ -// Copyright Dave Verwer, Sven A. Schmidt, and other contributors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import Foundation -import Plot - -enum SiteMapView { - - static var staticRoutes: [SiteURL] = [ - .home, - .addAPackage, - .faq, - .supporters, - .buildMonitor, - .privacy - ] - - 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 in - .sitemap( - .loc(SiteURL.package(.value(package.owner), - .value(package.repository), - .siteMap).absoluteURL()), - .unwrap(package.lastActivityAt, { .lastmod($0, timeZone: .utc) }) - ) - } - ) - ) - } - - static func staticPages() -> SiteMap { - SiteMap( - .group( - staticRoutes.map { page -> Node in - .url( - .loc(page.absoluteURL()) - ) - } - ) - ) - } - - 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 = 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) - }) - ) - } -} diff --git a/Tests/AppTests/PackageController+routesTests.swift b/Tests/AppTests/PackageController+routesTests.swift index e62c60cd1..ead6ecff7 100644 --- a/Tests/AppTests/PackageController+routesTests.swift +++ b/Tests/AppTests/PackageController+routesTests.swift @@ -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 diff --git a/Tests/AppTests/SitemapTests.swift b/Tests/AppTests/SitemapTests.swift index 00aa7f4ce..0f4e4a82f 100644 --- a/Tests/AppTests/SitemapTests.swift +++ b/Tests/AppTests/SitemapTests.swift @@ -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()) @@ -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")) @@ -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 @@ -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