From 3e5530e20ac57a9d130bc5731da0f0fe7c5e8667 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 12 Nov 2024 17:40:16 +0100 Subject: [PATCH 1/8] Add UpdateCustomCollectionAddKey migration --- .../083/UpdateCustomCollectionAddKey.swift | 36 +++++++++++++++++++ Sources/App/configure.swift | 3 ++ 2 files changed, 39 insertions(+) create mode 100644 Sources/App/Migrations/083/UpdateCustomCollectionAddKey.swift diff --git a/Sources/App/Migrations/083/UpdateCustomCollectionAddKey.swift b/Sources/App/Migrations/083/UpdateCustomCollectionAddKey.swift new file mode 100644 index 000000000..cff7fc1da --- /dev/null +++ b/Sources/App/Migrations/083/UpdateCustomCollectionAddKey.swift @@ -0,0 +1,36 @@ +// 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 Fluent + +struct UpdateCustomCollectionAddKey: AsyncMigration { + func prepare(on database: Database) async throws { + // We need to clear the custom_collections table, because the existing rows don't have + // `key` values and their existence prevents the update. + // They will be automatically regenerated by the next reconciliation run. + try await CustomCollection.query(on: database).delete() + try await database.schema("custom_collections") + .field("key", .string, .required) + .unique(on: "key") + .update() + } + + func revert(on database: Database) async throws { + try await database.schema("custom_collections") + .deleteField("key") + // The unique key constraint is automatically deleted when the field is deleted, + // no explicit clean up is required. + .update() + } +} diff --git a/Sources/App/configure.swift b/Sources/App/configure.swift index 173f3d935..9bbd93778 100644 --- a/Sources/App/configure.swift +++ b/Sources/App/configure.swift @@ -347,6 +347,9 @@ public func configure(_ app: Application) async throws -> String { do { // Migration 082 - Add `has_spi_badge` to `repositories` app.migrations.add(UpdateRepositoryAddHasSPIBadge()) } + do { // Migration 083 - Add `key` and unique constraint to `custom_collections` + app.migrations.add(UpdateCustomCollectionAddKey()) + } app.asyncCommands.use(Analyze.Command(), as: "analyze") app.asyncCommands.use(CreateRestfileCommand(), as: "create-restfile") From 04c910e95ef0a7ff1563e204193fe724fbad8bbc Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 12 Nov 2024 17:49:11 +0100 Subject: [PATCH 2/8] Add key field, filter by `key` in `findOrCreate` --- Sources/App/Models/CustomCollection.swift | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Sources/App/Models/CustomCollection.swift b/Sources/App/Models/CustomCollection.swift index 1f9b289c1..82a6fd75a 100644 --- a/Sources/App/Models/CustomCollection.swift +++ b/Sources/App/Models/CustomCollection.swift @@ -35,6 +35,9 @@ final class CustomCollection: @unchecked Sendable, Model, Content { // data fields + @Field(key: "key") + var key: String + @Field(key: "name") var name: String @@ -58,6 +61,7 @@ final class CustomCollection: @unchecked Sendable, Model, Content { self.id = id self.createdAt = createdAt self.updatedAt = updatedAt + self.key = details.key self.name = details.name self.description = details.description self.badge = details.badge @@ -68,6 +72,7 @@ final class CustomCollection: @unchecked Sendable, Model, Content { extension CustomCollection { struct Details: Codable, Equatable { + var key: String var name: String var description: String? var badge: String? @@ -76,7 +81,7 @@ extension CustomCollection { static func findOrCreate(on database: Database, _ details: Details) async throws -> CustomCollection { if let collection = try await CustomCollection.query(on: database) - .filter(\.$url == details.url) + .filter(\.$key == details.key) .first() { return collection } else { @@ -101,7 +106,7 @@ extension CustomCollection { } var details: Details { - .init(name: name, description: description, badge: badge, url: url) + .init(key: key, name: name, description: description, badge: badge, url: url) } } From 558a0a0a507d0095b40033a41749ef9d1aecc760 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 12 Nov 2024 17:50:00 +0100 Subject: [PATCH 3/8] Update tests --- Tests/AppTests/CustomCollectionTests.swift | 76 +++++++++++++++---- .../PackageCollectionControllerTests.swift | 4 +- Tests/AppTests/PackageCollectionTests.swift | 4 +- Tests/AppTests/ReconcilerTests.swift | 10 +-- Tests/AppTests/WebpageSnapshotTests.swift | 4 +- 5 files changed, 74 insertions(+), 24 deletions(-) diff --git a/Tests/AppTests/CustomCollectionTests.swift b/Tests/AppTests/CustomCollectionTests.swift index efd0d1ed9..38db2b44e 100644 --- a/Tests/AppTests/CustomCollectionTests.swift +++ b/Tests/AppTests/CustomCollectionTests.swift @@ -23,17 +23,33 @@ class CustomCollectionTests: AppTestCase { func test_CustomCollection_save() async throws { // MUT - try await CustomCollection(id: .id0, .init(name: "List", url: "https://github.com/foo/bar/list.json")) + try await CustomCollection(id: .id0, .init(key: "list", + name: "List", + url: "https://github.com/foo/bar/list.json")) .save(on: app.db) do { // validate let collection = try await CustomCollection.find(.id0, on: app.db).unwrap() + XCTAssertEqual(collection.key, "list") XCTAssertEqual(collection.name, "List") XCTAssertEqual(collection.url, "https://github.com/foo/bar/list.json") } + do { // ensure key is unique + try await CustomCollection(.init(key: "list", + name: "List 2", + url: "https://github.com/foo/bar/other-list.json")) + .save(on: app.db) + XCTFail("Expected failure") + } catch { + let msg = String(reflecting: error) + XCTAssert(msg.contains(#"duplicate key value violates unique constraint "uq:custom_collections.key""#), + "was: \(msg)") + } do { // ensure name is unique - try await CustomCollection(.init(name: "List", url: "https://github.com/foo/bar/other-list.json")) + try await CustomCollection(.init(key: "list-2", + name: "List", + url: "https://github.com/foo/bar/other-list.json")) .save(on: app.db) XCTFail("Expected failure") } catch { @@ -43,7 +59,9 @@ class CustomCollectionTests: AppTestCase { } do { // ensure url is unique - try await CustomCollection(.init(name: "List 2", url: "https://github.com/foo/bar/list.json")) + try await CustomCollection(.init(key: "list-2", + name: "List 2", + url: "https://github.com/foo/bar/list.json")) .save(on: app.db) XCTFail("Expected failure") } catch { @@ -56,28 +74,36 @@ class CustomCollectionTests: AppTestCase { func test_CustomCollection_findOrCreate() async throws { do { // initial call creates collection // MUT - let res = try await CustomCollection.findOrCreate(on: app.db, .init(name: "List", url: "url")) + let res = try await CustomCollection.findOrCreate(on: app.db, .init(key: "list", + name: "List", + url: "url")) // validate + XCTAssertEqual(res.key, "list") XCTAssertEqual(res.name, "List") XCTAssertEqual(res.url, "url") let c = try await CustomCollection.query(on: app.db).all() XCTAssertEqual(c.count, 1) + XCTAssertEqual(c.first?.key, "list") XCTAssertEqual(c.first?.name, "List") XCTAssertEqual(c.first?.url, "url") } do { // re-running is idempotent // MUT - let res = try await CustomCollection.findOrCreate(on: app.db, .init(name: "List", url: "url")) + let res = try await CustomCollection.findOrCreate(on: app.db, .init(key: "list", + name: "List", + url: "url")) // validate + XCTAssertEqual(res.key, "list") XCTAssertEqual(res.name, "List") XCTAssertEqual(res.url, "url") let c = try await CustomCollection.query(on: app.db).all() XCTAssertEqual(c.count, 1) + XCTAssertEqual(c.first?.key, "list") XCTAssertEqual(c.first?.name, "List") XCTAssertEqual(c.first?.url, "url") } @@ -87,7 +113,9 @@ class CustomCollectionTests: AppTestCase { // setup let pkg = Package(id: .id0, url: "1".asGithubUrl.url) try await pkg.save(on: app.db) - let collection = CustomCollection(id: .id1, .init(name: "List", url: "https://github.com/foo/bar/list.json")) + let collection = CustomCollection(id: .id1, .init(key: "list", + name: "List", + url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) // MUT @@ -102,7 +130,7 @@ class CustomCollectionTests: AppTestCase { XCTAssertEqual(pivot.package.url, "1".asGithubUrl) try await pivot.$customCollection.load(on: app.db) XCTAssertEqual(pivot.customCollection.id, .id1) - XCTAssertEqual(pivot.customCollection.name, "List") + XCTAssertEqual(pivot.customCollection.key, "list") } do { // ensure package is unique per list @@ -118,7 +146,9 @@ class CustomCollectionTests: AppTestCase { // setup let pkg = Package(id: .id0, url: "1".asGithubUrl.url) try await pkg.save(on: app.db) - let collection = CustomCollection(id: .id1, .init(name: "List", url: "https://github.com/foo/bar/list.json")) + let collection = CustomCollection(id: .id1, .init(key: "list", + name: "List", + url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) try await collection.$packages.attach(pkg, on: app.db) @@ -143,7 +173,9 @@ class CustomCollectionTests: AppTestCase { try await p1.save(on: app.db) let p2 = Package(id: .id1, url: "2".asGithubUrl.url) try await p2.save(on: app.db) - let collection = CustomCollection(id: .id2, .init(name: "List", url: "https://github.com/foo/bar/list.json")) + let collection = CustomCollection(id: .id2, .init(key: "list", + name: "List", + url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) try await collection.$packages.attach([p1, p2], on: app.db) @@ -161,12 +193,16 @@ class CustomCollectionTests: AppTestCase { let p1 = Package(id: .id0, url: "1".asGithubUrl.url) try await p1.save(on: app.db) do { - let collection = CustomCollection(id: .id1, .init(name: "List 1", url: "https://github.com/foo/bar/list-1.json")) + let collection = CustomCollection(id: .id1, .init(key: "list-1", + name: "List 1", + url: "https://github.com/foo/bar/list-1.json")) try await collection.save(on: app.db) try await collection.$packages.attach(p1, on: app.db) } do { - let collection = CustomCollection(id: .id2, .init(name: "List 2", url: "https://github.com/foo/bar/list-2.json")) + let collection = CustomCollection(id: .id2, .init(key: "list-2", + name: "List 2", + url: "https://github.com/foo/bar/list-2.json")) try await collection.save(on: app.db) try await collection.$packages.attach(p1, on: app.db) } @@ -182,7 +218,9 @@ class CustomCollectionTests: AppTestCase { // setup let pkg = Package(id: .id0, url: "1".asGithubUrl.url) try await pkg.save(on: app.db) - let collection = CustomCollection(id: .id1, .init(name: "List", url: "https://github.com/foo/bar/list.json")) + let collection = CustomCollection(id: .id1, .init(key: "list", + name: "List", + url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) try await collection.$packages.attach(pkg, on: app.db) do { @@ -220,7 +258,9 @@ class CustomCollectionTests: AppTestCase { // setup let pkg = Package(id: .id0, url: "1".asGithubUrl.url) try await pkg.save(on: app.db) - let collection = CustomCollection(id: .id1, .init(name: "List", url: "https://github.com/foo/bar/list.json")) + let collection = CustomCollection(id: .id1, .init(key: "list", + name: "List", + url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) try await collection.$packages.attach(pkg, on: app.db) do { @@ -256,7 +296,9 @@ class CustomCollectionTests: AppTestCase { func test_CustomCollection_reconcile() async throws { // Test reconciliation of a custom collection against a list of package URLs - let collection = CustomCollection(id: .id0, .init(name: "List", url: "https://github.com/foo/bar/list.json")) + let collection = CustomCollection(id: .id0, .init(key: "list", + name: "List", + url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) try await Package(id: .id1, url: URL("https://github.com/a.git")).save(on: app.db) try await Package(id: .id2, url: URL("https://github.com/b.git")).save(on: app.db) @@ -311,7 +353,9 @@ class CustomCollectionTests: AppTestCase { func test_CustomCollection_reconcile_caseSensitive() async throws { // Test reconciliation with a case-insensitive matching URL - let collection = CustomCollection(id: .id0, .init(name: "List", url: "https://github.com/foo/bar/list.json")) + let collection = CustomCollection(id: .id0, .init(key: "list", + name: "List", + url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) try await Package(id: .id1, url: URL("a")).save(on: app.db) @@ -321,7 +365,7 @@ class CustomCollectionTests: AppTestCase { do { // validate // The package is not added to the custom collection, because it is not an // exact match for the package URL. - // This is currently a limiting of the Fluent ~~ operator in the query + // This is currently a limitation of the Fluent ~~ operator in the query // filter(\.$url ~~ urls.map(\.absoluteString)) let count = try await CustomCollectionPackage.query(on: app.db).count() XCTAssertEqual(count, 0) diff --git a/Tests/AppTests/PackageCollectionControllerTests.swift b/Tests/AppTests/PackageCollectionControllerTests.swift index 09f31fb13..0d0bbe1d2 100644 --- a/Tests/AppTests/PackageCollectionControllerTests.swift +++ b/Tests/AppTests/PackageCollectionControllerTests.swift @@ -112,7 +112,9 @@ class PackageCollectionControllerTests: AppTestCase { licenseUrl: "https://foo/mit", owner: "foo", summary: "summary 1").create(on: app.db) - let collection = CustomCollection(id: .id2, .init(name: "Custom Collection", url: "https://github.com/foo/bar/list.json")) + let collection = CustomCollection(id: .id2, .init(key: "custom-collection", + name: "Custom Collection", + url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) try await collection.$packages.attach(p, on: app.db) diff --git a/Tests/AppTests/PackageCollectionTests.swift b/Tests/AppTests/PackageCollectionTests.swift index 31f789940..5b0ce4963 100644 --- a/Tests/AppTests/PackageCollectionTests.swift +++ b/Tests/AppTests/PackageCollectionTests.swift @@ -195,7 +195,9 @@ class PackageCollectionTests: AppTestCase { .save(on: app.db) return pkg } - let collection = CustomCollection(id: .id2, .init(name: "List", url: "https://github.com/foo/bar/list.json")) + let collection = CustomCollection(id: .id2, .init(key: "list", + name: "List", + url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) try await collection.$packages.attach([packages[0], packages[1]], on: app.db) diff --git a/Tests/AppTests/ReconcilerTests.swift b/Tests/AppTests/ReconcilerTests.swift index 2ea0dfc6a..ee8b537b4 100644 --- a/Tests/AppTests/ReconcilerTests.swift +++ b/Tests/AppTests/ReconcilerTests.swift @@ -143,7 +143,7 @@ class ReconcilerTests: AppTestCase { try await reconcileCustomCollection(client: app.client, database: app.db, fullPackageList: fullPackageList, - .init(name: "List", url: "url")) + .init(key: "list", name: "List", url: "url")) // validate let count = try await CustomCollection.query(on: app.db).count() @@ -161,7 +161,7 @@ class ReconcilerTests: AppTestCase { try await reconcileCustomCollection(client: app.client, database: app.db, fullPackageList: fullPackageList, - .init(name: "List", url: "url")) + .init(key: "list", name: "List", url: "url")) // validate let count = try await CustomCollection.query(on: app.db).count() @@ -181,7 +181,7 @@ class ReconcilerTests: AppTestCase { try await reconcileCustomCollection(client: app.client, database: app.db, fullPackageList: fullPackageList, - .init(name: "List", url: "url")) + .init(key: "list", name: "List", url: "url")) // validate let count = try await CustomCollection.query(on: app.db).count() @@ -207,7 +207,7 @@ class ReconcilerTests: AppTestCase { try await reconcileCustomCollection(client: app.client, database: app.db, fullPackageList: fullPackageList, - .init(name: "List", url: "url")) + .init(key: "list", name: "List", url: "url")) // validate let collection = try await CustomCollection.query(on: app.db).first().unwrap() @@ -233,7 +233,7 @@ class ReconcilerTests: AppTestCase { } } $0.packageListRepository.fetchCustomCollections = { @Sendable _ in - [.init(name: "List", url: "collectionURL")] + [.init(key: "list", name: "List", url: "collectionURL")] } } operation: { // MUT diff --git a/Tests/AppTests/WebpageSnapshotTests.swift b/Tests/AppTests/WebpageSnapshotTests.swift index ff544f0f9..1801325c3 100644 --- a/Tests/AppTests/WebpageSnapshotTests.swift +++ b/Tests/AppTests/WebpageSnapshotTests.swift @@ -296,7 +296,9 @@ class WebpageSnapshotTests: SnapshotTestCase { func test_PackageShowView_customCollection() throws { var model = API.PackageController.GetRoute.Model.mock model.homepageUrl = "https://swiftpackageindex.com/" - model.customCollections = [.init(name: "Custom Collection", url: "https://github.com/foo/bar/list.json")] + model.customCollections = [.init(key: "custom-collection", + name: "Custom Collection", + url: "https://github.com/foo/bar/list.json")] let page = { PackageShow.View(path: "", model: model, packageSchema: .mock).document() } assertSnapshot(of: page, as: .html) From 0c5eb018cfc792e04433c844939e59b91f7940ba Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 12 Nov 2024 18:23:13 +0100 Subject: [PATCH 4/8] Update the collection if details have changed --- Sources/App/Models/CustomCollection.swift | 20 +++++++++++++++++++- Tests/AppTests/CustomCollectionTests.swift | 17 +++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/Sources/App/Models/CustomCollection.swift b/Sources/App/Models/CustomCollection.swift index 82a6fd75a..3cac38444 100644 --- a/Sources/App/Models/CustomCollection.swift +++ b/Sources/App/Models/CustomCollection.swift @@ -83,6 +83,11 @@ extension CustomCollection { if let collection = try await CustomCollection.query(on: database) .filter(\.$key == details.key) .first() { + if collection.details != details { + // Update the collection if any of the details have changed + collection.details = details + try await collection.update(on: database) + } return collection } else { let collection = CustomCollection(details) @@ -106,7 +111,20 @@ extension CustomCollection { } var details: Details { - .init(key: key, name: name, description: description, badge: badge, url: url) + get { + .init(key: key, + name: name, + description: description, + badge: badge, + url: url) + } + set { + key = newValue.key + name = newValue.name + description = newValue.description + badge = newValue.badge + url = newValue.url + } } } diff --git a/Tests/AppTests/CustomCollectionTests.swift b/Tests/AppTests/CustomCollectionTests.swift index 38db2b44e..8df24a845 100644 --- a/Tests/AppTests/CustomCollectionTests.swift +++ b/Tests/AppTests/CustomCollectionTests.swift @@ -107,6 +107,23 @@ class CustomCollectionTests: AppTestCase { XCTAssertEqual(c.first?.name, "List") XCTAssertEqual(c.first?.url, "url") } + + do { // a record is updated if data has changed + // MUT + let res = try await CustomCollection.findOrCreate(on: app.db, .init(key: "list", + name: "New name", + url: "new-url")) + // validate + XCTAssertEqual(res.key, "list") + XCTAssertEqual(res.name, "New name") + XCTAssertEqual(res.url, "new-url") + + let c = try await CustomCollection.query(on: app.db).all() + XCTAssertEqual(c.count, 1) + XCTAssertEqual(c.first?.key, "list") + XCTAssertEqual(c.first?.name, "New name") + XCTAssertEqual(c.first?.url, "new-url") + } } func test_CustomCollectionPackage_attach() async throws { From 2c14d7ce796e4be57dcb1b45b3530406748917c1 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 12 Nov 2024 18:38:41 +0100 Subject: [PATCH 5/8] Update test_CustomCollection_reconcile_caseInsensitive --- Tests/AppTests/CustomCollectionTests.swift | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/Tests/AppTests/CustomCollectionTests.swift b/Tests/AppTests/CustomCollectionTests.swift index 8df24a845..b2b58a24f 100644 --- a/Tests/AppTests/CustomCollectionTests.swift +++ b/Tests/AppTests/CustomCollectionTests.swift @@ -368,27 +368,24 @@ class CustomCollectionTests: AppTestCase { } } - func test_CustomCollection_reconcile_caseSensitive() async throws { + func test_CustomCollection_reconcile_caseInsensitive() async throws { // Test reconciliation with a case-insensitive matching URL let collection = CustomCollection(id: .id0, .init(key: "list", name: "List", url: "https://github.com/foo/bar/list.json")) try await collection.save(on: app.db) - try await Package(id: .id1, url: URL("a")).save(on: app.db) + try await Package(id: .id1, url: URL("https://github.com/a.git")).save(on: app.db) // MUT - try await collection.reconcile(on: app.db, packageURLs: [URL("A")]) + try await collection.reconcile(on: app.db, packageURLs: [URL("https://github.com/A.git")]) do { // validate - // The package is not added to the custom collection, because it is not an - // exact match for the package URL. - // This is currently a limitation of the Fluent ~~ operator in the query - // filter(\.$url ~~ urls.map(\.absoluteString)) + // The package is added to the custom collection via case-insensitive match let count = try await CustomCollectionPackage.query(on: app.db).count() - XCTAssertEqual(count, 0) + XCTAssertEqual(count, 1) let collection = try await CustomCollection.find(.id0, on: app.db).unwrap() try await collection.$packages.load(on: app.db) - XCTAssertEqual(collection.packages.map(\.url), []) + XCTAssertEqual(collection.packages.map(\.url), ["https://github.com/a.git"]) } } From 3ad1dd15e9b1843ae52e8c8076c99eed608854f5 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 13 Nov 2024 15:43:53 +0100 Subject: [PATCH 6/8] Use key instead of name for lookups and urls --- .../CustomCollectionsController.swift | 15 ++++++++++----- Sources/App/Core/SiteURL.swift | 18 +++++++++--------- .../CustomCollectionShow+Model.swift | 5 ++++- .../CustomCollectionShow+View.swift | 2 +- .../PackageController/GetRoute.Model+ext.swift | 2 +- .../Mocks/CustomCollectionShow+mock.swift | 5 ++++- .../test_CustomCollectionShow.1.html | 18 +++++++++--------- ...est_PackageShowView_customCollection.1.html | 2 +- 8 files changed, 39 insertions(+), 28 deletions(-) diff --git a/Sources/App/Controllers/CustomCollectionsController.swift b/Sources/App/Controllers/CustomCollectionsController.swift index 91e1fc0ca..dac647e72 100644 --- a/Sources/App/Controllers/CustomCollectionsController.swift +++ b/Sources/App/Controllers/CustomCollectionsController.swift @@ -19,7 +19,7 @@ import Vapor enum CustomCollectionsController { - static func query(on database: Database, name: String, page: Int, pageSize: Int) async throws -> Page> { + static func query(on database: Database, key: String, page: Int, pageSize: Int) async throws -> Page> { try await Joined3 .query(on: database, version: .defaultBranch) .join(CustomCollectionPackage.self, on: \Package.$id == \CustomCollectionPackage.$package.$id) @@ -30,7 +30,7 @@ enum CustomCollectionsController { .field(Repository.self, \.$stars) .field(Repository.self, \.$summary) .field(Version.self, \.$packageName) - .filter(CustomCollection.self, \.$name == name) + .filter(CustomCollection.self, \.$key == key) .sort(Repository.self, \.$name) .page(page, size: pageSize) } @@ -56,11 +56,15 @@ enum CustomCollectionsController { @Sendable static func show(req: Request) async throws -> HTML { - guard let name = req.parameters.get("name") else { + guard let key = req.parameters.get("key") else { throw Abort(.notFound) } let query = try req.query.decode(Query.self) - let page = try await Self.query(on: req.db, name: name, page: query.page, pageSize: query.pageSize) + let collection = try await CustomCollection.query(on: req.db) + .filter(\.$key == key) + .first() + .unwrap(or: Abort(.notFound)) + let page = try await Self.query(on: req.db, key: key, page: query.page, pageSize: query.pageSize) guard !page.results.isEmpty else { throw Abort(.notFound) @@ -69,7 +73,8 @@ enum CustomCollectionsController { let packageInfo = page.results.compactMap(PackageInfo.init(package:)) let model = CustomCollectionShow.Model( - name: name, + key: collection.key, + name: collection.name, packages: packageInfo, page: query.page, hasMoreResults: page.hasMoreResults diff --git a/Sources/App/Core/SiteURL.swift b/Sources/App/Core/SiteURL.swift index e71a5a46a..86c48fc6a 100644 --- a/Sources/App/Core/SiteURL.swift +++ b/Sources/App/Core/SiteURL.swift @@ -113,7 +113,7 @@ enum SiteURL: Resourceable, Sendable { case blogPost(_ slug: Parameter) case buildMonitor case builds(_ id: Parameter) - case collections(_ name: Parameter) + case collections(_ key: Parameter) case docs(Docs) case faq case home @@ -122,7 +122,7 @@ enum SiteURL: Resourceable, Sendable { case keywords(_ keyword: Parameter) case package(_ owner: Parameter, _ repository: Parameter, PackagePathComponents?) case packageCollectionAuthor(_ owner: Parameter) - case packageCollectionCustom(_ name: Parameter) + case packageCollectionCustom(_ key: Parameter) case packageCollections case privacy case readyForSwift6 @@ -171,11 +171,11 @@ enum SiteURL: Resourceable, Sendable { case .buildMonitor: return "build-monitor" - case let .collections(.value(name)): - return "collections/\(name.urlPathEncoded)" + case let .collections(.value(key)): + return "collections/\(key.urlPathEncoded)" case .collections(.key): - fatalError("path must not be called with a name parameter") + fatalError("invalid path: \(self)") case let .docs(next): return "docs/\(next.path)" @@ -215,8 +215,8 @@ enum SiteURL: Resourceable, Sendable { case .packageCollectionAuthor(.key): fatalError("invalid path: \(self)") - case let .packageCollectionCustom(.value(name)): - return "collections/\(name.urlPathEncoded)/collection.json" + case let .packageCollectionCustom(.value(key)): + return "collections/\(key.urlPathEncoded)/collection.json" case .packageCollectionCustom(.key): fatalError("invalid path: \(self)") @@ -298,7 +298,7 @@ enum SiteURL: Resourceable, Sendable { fatalError("pathComponents must not be called with a value parameter") case .collections(.key): - return ["collections", ":name"] + return ["collections", ":key"] case .collections(.value): fatalError("pathComponents must not be called with a value parameter") @@ -325,7 +325,7 @@ enum SiteURL: Resourceable, Sendable { fatalError("pathComponents must not be called with a value parameter") case .packageCollectionCustom(.key): - return ["collections", ":name", "collection.json"] + return ["collections", ":key", "collection.json"] case .packageCollectionCustom(.value): fatalError("pathComponents must not be called with a value parameter") diff --git a/Sources/App/Views/CustomCollection/CustomCollectionShow+Model.swift b/Sources/App/Views/CustomCollection/CustomCollectionShow+Model.swift index a4334ea20..751bac799 100644 --- a/Sources/App/Views/CustomCollection/CustomCollectionShow+Model.swift +++ b/Sources/App/Views/CustomCollection/CustomCollectionShow+Model.swift @@ -14,15 +14,18 @@ extension CustomCollectionShow { struct Model { + var key: String var name: String var packages: [PackageInfo] var page: Int var hasMoreResults: Bool - internal init(name: String, + internal init(key: String, + name: String, packages: [PackageInfo], page: Int, hasMoreResults: Bool) { + self.key = key self.name = name self.packages = packages self.page = page diff --git a/Sources/App/Views/CustomCollection/CustomCollectionShow+View.swift b/Sources/App/Views/CustomCollection/CustomCollectionShow+View.swift index bbe6f09ee..00eb2c673 100644 --- a/Sources/App/Views/CustomCollection/CustomCollectionShow+View.swift +++ b/Sources/App/Views/CustomCollection/CustomCollectionShow+View.swift @@ -58,7 +58,7 @@ enum CustomCollectionShow { ), .copyableInputForm(buttonName: "Copy Package Collection URL", eventName: "Copy Package Collection URL Button", - valueToCopy: SiteURL.packageCollectionCustom(.value(model.name)).absoluteURL()), + valueToCopy: SiteURL.packageCollectionCustom(.value(model.key)).absoluteURL()), .hr(.class("minor")), .ul( .id("package-list"), diff --git a/Sources/App/Views/PackageController/GetRoute.Model+ext.swift b/Sources/App/Views/PackageController/GetRoute.Model+ext.swift index ea59e2b17..2c8ee3aea 100644 --- a/Sources/App/Views/PackageController/GetRoute.Model+ext.swift +++ b/Sources/App/Views/PackageController/GetRoute.Model+ext.swift @@ -404,7 +404,7 @@ extension API.PackageController.GetRoute.Model { .class("custom-collections"), .forEach(customCollections, { collection in .a( - .href(SiteURL.collections(.value(collection.name)).relativeURL()), + .href(SiteURL.collections(.value(collection.key)).relativeURL()), .text("\(collection.name)") ) }) diff --git a/Tests/AppTests/Mocks/CustomCollectionShow+mock.swift b/Tests/AppTests/Mocks/CustomCollectionShow+mock.swift index c42c7a9ae..7179ef43f 100644 --- a/Tests/AppTests/Mocks/CustomCollectionShow+mock.swift +++ b/Tests/AppTests/Mocks/CustomCollectionShow+mock.swift @@ -28,6 +28,9 @@ extension CustomCollectionShow.Model { stars: 4, lastActivityAt: .t0 ) } - return .init(name: "Some Collection", packages: packages, page: 1, hasMoreResults: false) + return .init(key: "custom-collection", + name: "Custom Collection", + packages: packages, page: 1, + hasMoreResults: false) } } diff --git a/Tests/AppTests/__Snapshots__/WebpageSnapshotTests/test_CustomCollectionShow.1.html b/Tests/AppTests/__Snapshots__/WebpageSnapshotTests/test_CustomCollectionShow.1.html index fc407c80d..cc119aa3a 100644 --- a/Tests/AppTests/__Snapshots__/WebpageSnapshotTests/test_CustomCollectionShow.1.html +++ b/Tests/AppTests/__Snapshots__/WebpageSnapshotTests/test_CustomCollectionShow.1.html @@ -7,12 +7,12 @@ - Packages for collection Some Collection – Swift Package Index - - - - - + Packages for collection Custom Collection – Swift Package Index + + + + + @@ -75,19 +75,19 @@

  • - Some Collection + Custom Collection
    • -

    Packages for collection “Some Collection”

    +

    Packages for collection “Custom Collection”

    These packages are available as a package collection, usable in Xcode 13 or the Swift Package Manager 5.5.

    - +
      diff --git a/Tests/AppTests/__Snapshots__/WebpageSnapshotTests/test_PackageShowView_customCollection.1.html b/Tests/AppTests/__Snapshots__/WebpageSnapshotTests/test_PackageShowView_customCollection.1.html index a988d08a4..41d1a5581 100644 --- a/Tests/AppTests/__Snapshots__/WebpageSnapshotTests/test_PackageShowView_customCollection.1.html +++ b/Tests/AppTests/__Snapshots__/WebpageSnapshotTests/test_PackageShowView_customCollection.1.html @@ -169,7 +169,7 @@

      When working with a Swift Package Manager manifest:

    • No plugins
    • 2 macros
    • - Custom Collection + Custom Collection
    From 729f4e1eae06a329a6e9c6501dfe853dd1939a17 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 13 Nov 2024 16:11:40 +0100 Subject: [PATCH 7/8] Update CustomCollectionsController to use key instead of name --- .../App/Controllers/CustomCollectionsController.swift | 4 +--- .../App/Controllers/PackageCollectionController.swift | 10 ++++++---- Sources/App/Core/PackageCollection+VersionResult.swift | 4 ++-- Sources/App/Models/CustomCollection.swift | 10 +++++++--- Tests/AppTests/PackageCollectionControllerTests.swift | 2 +- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/Sources/App/Controllers/CustomCollectionsController.swift b/Sources/App/Controllers/CustomCollectionsController.swift index dac647e72..a1d0e8e76 100644 --- a/Sources/App/Controllers/CustomCollectionsController.swift +++ b/Sources/App/Controllers/CustomCollectionsController.swift @@ -60,9 +60,7 @@ enum CustomCollectionsController { throw Abort(.notFound) } let query = try req.query.decode(Query.self) - let collection = try await CustomCollection.query(on: req.db) - .filter(\.$key == key) - .first() + let collection = try await CustomCollection.find(on: req.db, key: key) .unwrap(or: Abort(.notFound)) let page = try await Self.query(on: req.db, key: key, page: query.page, pageSize: query.pageSize) diff --git a/Sources/App/Controllers/PackageCollectionController.swift b/Sources/App/Controllers/PackageCollectionController.swift index 171f4e584..867d38226 100644 --- a/Sources/App/Controllers/PackageCollectionController.swift +++ b/Sources/App/Controllers/PackageCollectionController.swift @@ -32,12 +32,14 @@ enum PackageCollectionController { filterBy: .author(owner), authorName: "\(owner) via the Swift Package Index" ) - case let .custom(name): + case let .custom(key): + let collection = try await CustomCollection.find(on: req.db, key: key) + .unwrap(or: Abort(.notFound)) return try await SignedCollection.generate( db: req.db, - filterBy: .customCollection(name), + filterBy: .customCollection(key), authorName: "Swift Package Index", - collectionName: name, + collectionName: collection.name, overview: "A custom package collection generated by the Swift Package Index" ) } @@ -53,7 +55,7 @@ enum PackageCollectionController { static func getCollectionType(req: Request) -> CollectionType? { if let owner = req.parameters.get("owner") { return .author(owner) } - if let name = req.parameters.get("name") { return .custom(name) } + if let key = req.parameters.get("key") { return .custom(key) } return nil } } diff --git a/Sources/App/Core/PackageCollection+VersionResult.swift b/Sources/App/Core/PackageCollection+VersionResult.swift index ac3c69e9d..f8078fbd9 100644 --- a/Sources/App/Core/PackageCollection+VersionResult.swift +++ b/Sources/App/Core/PackageCollection+VersionResult.swift @@ -55,11 +55,11 @@ extension PackageCollection.VersionResult { switch filter { case let .author(owner): query.filter(Repository.self, \Repository.$owner, .custom("ilike"), owner) - case let .customCollection(name): + case let .customCollection(key): query .join(CustomCollectionPackage.self, on: \Package.$id == \CustomCollectionPackage.$package.$id) .join(CustomCollection.self, on: \CustomCollection.$id == \CustomCollectionPackage.$customCollection.$id) - .filter(CustomCollection.self, \.$name == name) + .filter(CustomCollection.self, \.$key == key) case let .urls(packageURLs): query.filter(App.Package.self, \.$url ~~ packageURLs) } diff --git a/Sources/App/Models/CustomCollection.swift b/Sources/App/Models/CustomCollection.swift index 3cac38444..54711b4c7 100644 --- a/Sources/App/Models/CustomCollection.swift +++ b/Sources/App/Models/CustomCollection.swift @@ -79,10 +79,14 @@ extension CustomCollection { var url: URL } + static func find(on database: Database, key: String) async throws -> CustomCollection? { + try await CustomCollection.query(on: database) + .filter(\.$key == key) + .first() + } + static func findOrCreate(on database: Database, _ details: Details) async throws -> CustomCollection { - if let collection = try await CustomCollection.query(on: database) - .filter(\.$key == details.key) - .first() { + if let collection = try await CustomCollection.find(on: database, key: details.key) { if collection.details != details { // Update the collection if any of the details have changed collection.details = details diff --git a/Tests/AppTests/PackageCollectionControllerTests.swift b/Tests/AppTests/PackageCollectionControllerTests.swift index 0d0bbe1d2..664664938 100644 --- a/Tests/AppTests/PackageCollectionControllerTests.swift +++ b/Tests/AppTests/PackageCollectionControllerTests.swift @@ -122,7 +122,7 @@ class PackageCollectionControllerTests: AppTestCase { let encoder = self.encoder try await app.test( .GET, - "collections/Custom%20Collection/collection.json", + "collections/custom-collection/collection.json", afterResponse: { @MainActor res async throws in // validation XCTAssertEqual(res.status, .ok) From bb6fdacd17dc0af4ec065668bf2e8b5658c569b1 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 13 Nov 2024 16:25:13 +0100 Subject: [PATCH 8/8] Update tests --- Tests/AppTests/PackageCollectionTests.swift | 2 +- Tests/AppTests/SiteURLTests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/AppTests/PackageCollectionTests.swift b/Tests/AppTests/PackageCollectionTests.swift index 5b0ce4963..012852849 100644 --- a/Tests/AppTests/PackageCollectionTests.swift +++ b/Tests/AppTests/PackageCollectionTests.swift @@ -203,7 +203,7 @@ class PackageCollectionTests: AppTestCase { // MUT let res = try await VersionResult.query(on: self.app.db, - filterBy: .customCollection("List")) + filterBy: .customCollection("list")) // validate selection (relationship loading is tested in test_query_filter_urls) XCTAssertEqual(res.map(\.version.packageName), diff --git a/Tests/AppTests/SiteURLTests.swift b/Tests/AppTests/SiteURLTests.swift index 3f43ffadb..ac55fa533 100644 --- a/Tests/AppTests/SiteURLTests.swift +++ b/Tests/AppTests/SiteURLTests.swift @@ -170,7 +170,7 @@ class SiteURLTests: XCTestCase { func test_collections() throws { XCTAssertEqual(SiteURL.collections(.value("foo")).path, "collections/foo") - XCTAssertEqual(SiteURL.collections(.key).pathComponents.map(\.description), ["collections", ":name"]) + XCTAssertEqual(SiteURL.collections(.key).pathComponents.map(\.description), ["collections", ":key"]) } }