Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 3469 dependency transition 8 #3525

Merged
merged 2 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions Sources/App/Core/AppEnvironment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import FoundationNetworking


struct AppEnvironment: Sendable {
var currentReferenceCache: @Sendable () -> CurrentReferenceCache?
var dbId: @Sendable () -> String?
var fetchDocumentation: @Sendable (_ client: Client, _ url: URI) async throws -> ClientResponse
var fetchHTTPStatusCode: @Sendable (_ url: String) async throws -> HTTPStatus
var fetchLicense: @Sendable (_ client: Client, _ owner: String, _ repository: String) async -> Github.License?
Expand Down Expand Up @@ -87,8 +85,6 @@ extension AppEnvironment {
nonisolated(unsafe) static var logger: Logger!

static let live = AppEnvironment(
currentReferenceCache: { .live },
dbId: { Environment.get("DATABASE_ID") },
fetchDocumentation: { client, url in try await client.get(url) },
fetchHTTPStatusCode: { url in try await Networking.fetchHTTPStatusCode(url) },
fetchLicense: { client, owner, repo in await Github.fetchLicense(client:client, owner: owner, repository: repo) },
Expand Down
4 changes: 4 additions & 0 deletions Sources/App/Core/Dependencies/EnvironmentClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ struct EnvironmentClient {
var collectionSigningCertificateChain: @Sendable () -> [URL] = { XCTFail("collectionSigningCertificateChain"); return [] }
var collectionSigningPrivateKey: @Sendable () -> Data?
var current: @Sendable () -> Environment = { XCTFail("current"); return .development }
var currentReferenceCache: @Sendable () -> CurrentReferenceCache?
var dbId: @Sendable () -> String?
var mastodonCredentials: @Sendable () -> Mastodon.Credentials?
var mastodonPost: @Sendable (_ client: Client, _ post: String) async throws -> Void
var random: @Sendable (_ range: ClosedRange<Double>) -> Double = { XCTFail("random"); return Double.random(in: $0) }
Expand Down Expand Up @@ -91,6 +93,8 @@ extension EnvironmentClient: DependencyKey {
Environment.get("COLLECTION_SIGNING_PRIVATE_KEY").map { Data($0.utf8) }
},
current: { (try? Environment.detect()) ?? .development },
currentReferenceCache: { .live },
dbId: { Environment.get("DATABASE_ID") },
mastodonCredentials: {
Environment.get("MASTODON_ACCESS_TOKEN")
.map(Mastodon.Credentials.init(accessToken:))
Expand Down
2 changes: 1 addition & 1 deletion Sources/App/Views/PublicPage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class PublicPage {
return HTML(
.lang(.english),
.comment("Version: \(environment.appVersion())"),
.comment("DB Id: \(Current.dbId())"),
.comment("DB Id: \(environment.dbId())"),
head(),
body()
)
Expand Down
14 changes: 8 additions & 6 deletions Sources/App/routes+documentation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.


import Dependencies
import Vapor


Expand Down Expand Up @@ -124,7 +124,7 @@ private extension Parameters {
}
}
}


struct DocRedirect {
var owner: String
Expand Down Expand Up @@ -152,15 +152,15 @@ extension Request {
target = try await DocumentationTarget.query(on: db, owner: owner, repository: repository,
docVersion: .reference(ref))
}

case .none:
target = try await DocumentationTarget.query(on: db, owner: owner, repository: repository)
}
guard let target else { throw Abort(.notFound) }

return .init(owner: owner, repository: repository, target: target, path: path)
}

func getDocRoute(fragment: DocRoute.Fragment) async throws -> DocRoute {
guard let owner = parameters.get("owner"),
let repository = parameters.get("repository"),
Expand All @@ -170,16 +170,18 @@ extension Request {
if fragment.requiresArchive && archive == nil { throw Abort(.badRequest) }
let pathElements = parameters.pathElements(for: fragment, archive: archive)

@Dependency(\.environment) var environment

let docVersion = try await { () -> DocVersion in
if reference == String.current {
if let ref = Current.currentReferenceCache()?[owner: owner, repository: repository] {
if let ref = environment.currentReferenceCache()?[owner: owner, repository: repository] {
return .current(referencing: ref)
}

guard let params = try await DocumentationTarget.query(on: db, owner: owner, repository: repository)?.internal
else { throw Abort(.notFound) }

Current.currentReferenceCache()?[owner: owner, repository: repository] = "\(params.docVersion)"
environment.currentReferenceCache()?[owner: owner, repository: repository] = "\(params.docVersion)"
return .current(referencing: "\(params.docVersion)")
} else {
return .reference(reference)
Expand Down
7 changes: 7 additions & 0 deletions Tests/AppTests/ApiTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class ApiTests: AppTestCase {
func test_search_unauthenticated() async throws {
try await withDependencies {
$0.environment.apiSigningKey = { "secret" }
$0.environment.dbId = { nil }
} operation: {
// MUT
try await app.test(.GET, "api/search?query=test",
Expand Down Expand Up @@ -344,6 +345,7 @@ class ApiTests: AppTestCase {
// Ensure unauthenticated access raises a 401
try await withDependencies {
$0.environment.builderToken = { "secr3t" }
$0.environment.dbId = { nil }
} operation: {
// setup
let p = try await savePackage(on: app.db, "1")
Expand Down Expand Up @@ -640,6 +642,7 @@ class ApiTests: AppTestCase {
func test_post_docReport_non_existing_build() async throws {
try await withDependencies {
$0.environment.builderToken = { "secr3t" }
$0.environment.dbId = { nil }
} operation: {
// setup
let nonExistingBuildId = UUID()
Expand All @@ -665,6 +668,7 @@ class ApiTests: AppTestCase {
func test_post_docReport_unauthenticated() async throws {
try await withDependencies {
$0.environment.builderToken = { "secr3t" }
$0.environment.dbId = { nil }
} operation: {
// setup
let p = try await savePackage(on: app.db, "1")
Expand Down Expand Up @@ -978,6 +982,7 @@ class ApiTests: AppTestCase {
func test_package_collections_packageURLs_limit() throws {
try withDependencies {
$0.environment.apiSigningKey = { "secret" }
$0.environment.dbId = { nil }
} operation: {
let dto = API.PostPackageCollectionDTO(
// request 21 urls - this should raise a 400
Expand All @@ -999,6 +1004,7 @@ class ApiTests: AppTestCase {
func test_package_collections_unauthorized() throws {
try withDependencies {
$0.environment.apiSigningKey = { "secret" }
$0.environment.dbId = { nil }
} operation: {
// MUT - happy path
let body: ByteBuffer = .init(string: """
Expand Down Expand Up @@ -1042,6 +1048,7 @@ class ApiTests: AppTestCase {
func test_packages_get() async throws {
try await withDependencies {
$0.environment.apiSigningKey = { "secret" }
$0.environment.dbId = { nil }
} operation: {
let owner = "owner"
let repo = "repo"
Expand Down
43 changes: 26 additions & 17 deletions Tests/AppTests/AuthorControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import XCTest

@testable import App

import Dependencies
import Vapor
import XCTest


class AuthorControllerTests: AppTestCase {

Expand Down Expand Up @@ -66,27 +69,33 @@ class AuthorControllerTests: AppTestCase {
}

func test_show_owner() async throws {
// setup
let p = try await savePackage(on: app.db, "1")
try await Repository(package: p, owner: "owner").save(on: app.db)
try await Version(package: p, latest: .defaultBranch).save(on: app.db)
try await withDependencies {
$0.environment.dbId = { nil }
} operation: {
let p = try await savePackage(on: app.db, "1")
try await Repository(package: p, owner: "owner").save(on: app.db)
try await Version(package: p, latest: .defaultBranch).save(on: app.db)

// MUT
try await app.test(.GET, "/owner", afterResponse: { response async in
XCTAssertEqual(response.status, .ok)
})
// MUT
try await app.test(.GET, "/owner", afterResponse: { response async in
XCTAssertEqual(response.status, .ok)
})
}
}

func test_show_owner_empty() async throws {
// setup
let p = try await savePackage(on: app.db, "1")
try await Repository(package: p, owner: "owner").save(on: app.db)
try await Version(package: p, latest: .defaultBranch).save(on: app.db)
try await withDependencies {
$0.environment.dbId = { nil }
} operation: {
let p = try await savePackage(on: app.db, "1")
try await Repository(package: p, owner: "owner").save(on: app.db)
try await Version(package: p, latest: .defaultBranch).save(on: app.db)

// MUT
try await app.test(.GET, "/fake-owner", afterResponse: { response async in
XCTAssertEqual(response.status, .notFound)
})
// MUT
try await app.test(.GET, "/fake-owner", afterResponse: { response async in
XCTAssertEqual(response.status, .notFound)
})
}
}

}
1 change: 1 addition & 0 deletions Tests/AppTests/BuildMonitorControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class BuildMonitorControllerTests: AppTestCase {
func test_show_owner() async throws {
try await withDependencies {
$0.date.now = .now
$0.environment.dbId = { nil }
} operation: {
let package = try await savePackage(on: app.db, "https://github.com/daveverwer/LeftPad")
let version = try Version(package: package)
Expand Down
36 changes: 23 additions & 13 deletions Tests/AppTests/ErrorMiddlewareTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import XCTest

@testable import App

import Dependencies
import Vapor
import XCTest


class ErrorMiddlewareTests: AppTestCase {
Expand All @@ -39,24 +41,32 @@ class ErrorMiddlewareTests: AppTestCase {

func test_html_error() throws {
// Test to ensure errors are converted to html error pages via the ErrorMiddleware
try app.test(.GET, "404", afterResponse: { response in
XCTAssertEqual(response.content.contentType, .html)
XCTAssert(response.body.asString().contains("404 - Not Found"))
})
try withDependencies {
$0.environment.dbId = { nil }
} operation: {
try app.test(.GET, "404", afterResponse: { response in
XCTAssertEqual(response.content.contentType, .html)
XCTAssert(response.body.asString().contains("404 - Not Found"))
})
}
}

func test_status_code() throws {
// Ensure we're still reporting the actual status code even when serving html pages
// (Status is important for Google ranking, see
// https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/323)
try app.test(.GET, "404", afterResponse: { response in
XCTAssertEqual(response.status, .notFound)
XCTAssertEqual(response.content.contentType, .html)
})
try app.test(.GET, "500", afterResponse: { response in
XCTAssertEqual(response.status, .internalServerError)
XCTAssertEqual(response.content.contentType, .html)
})
try withDependencies {
$0.environment.dbId = { nil }
} operation: {
try app.test(.GET, "404", afterResponse: { response in
XCTAssertEqual(response.status, .notFound)
XCTAssertEqual(response.content.contentType, .html)
})
try app.test(.GET, "500", afterResponse: { response in
XCTAssertEqual(response.status, .internalServerError)
XCTAssertEqual(response.content.contentType, .html)
})
}
}

}
42 changes: 26 additions & 16 deletions Tests/AppTests/KeywordControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import XCTest

@testable import App

import Dependencies
import Vapor
import XCTest


class KeywordControllerTests: AppTestCase {

Expand Down Expand Up @@ -102,25 +105,32 @@ class KeywordControllerTests: AppTestCase {
}

func test_show_keyword() async throws {
// setup
do {
let p = try await savePackage(on: app.db, "1")
try await Repository(package: p,
keywords: ["foo"],
name: "1",
owner: "owner").save(on: app.db)
try await Version(package: p, latest: .defaultBranch).save(on: app.db)
}
// MUT
try await app.test(.GET, "/keywords/foo") { req async in
// validate
XCTAssertEqual(req.status, .ok)
try await withDependencies {
$0.environment.dbId = { nil }
} operation: {
do {
let p = try await savePackage(on: app.db, "1")
try await Repository(package: p,
keywords: ["foo"],
name: "1",
owner: "owner").save(on: app.db)
try await Version(package: p, latest: .defaultBranch).save(on: app.db)
}
// MUT
try await app.test(.GET, "/keywords/foo") { req async in
// validate
XCTAssertEqual(req.status, .ok)
}
}
}

func test_not_found() throws {
try app.test(.GET, "/keywords/baz") {
XCTAssertEqual($0.status, .notFound)
try withDependencies {
$0.environment.dbId = { nil }
} operation: {
try app.test(.GET, "/keywords/baz") {
XCTAssertEqual($0.status, .notFound)
}
}
}

Expand Down
2 changes: 0 additions & 2 deletions Tests/AppTests/Mocks/AppEnvironment+mock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import Vapor
extension AppEnvironment {
static func mock(eventLoop: EventLoop) -> Self {
.init(
currentReferenceCache: { nil },
dbId: { "db-id" },
fetchDocumentation: { _, _ in .init(status: .ok) },
fetchHTTPStatusCode: { _ in .ok },
fetchLicense: { _, _, _ in .init(htmlUrl: "https://github.com/foo/bar/blob/main/LICENSE") },
Expand Down
20 changes: 12 additions & 8 deletions Tests/AppTests/PackageCollectionControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,18 @@ class PackageCollectionControllerTests: AppTestCase {

func test_nonexisting_404() throws {
// Ensure a request for a non-existing collection returns a 404
// MUT
try app.test(
.GET,
"foo/collection.json",
afterResponse: { res in
// validation
XCTAssertEqual(res.status, .notFound)
})
try withDependencies {
$0.environment.dbId = { nil }
} operation: {
// MUT
try app.test(
.GET,
"foo/collection.json",
afterResponse: { res in
// validation
XCTAssertEqual(res.status, .notFound)
})
}
}

}
Expand Down
Loading
Loading