Merge pull request #3494 from SwiftPackageIndex/custom-collections-tweaks

Custom collections tweaks

Author: finestructure

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<Joined3<Package, Repository, Version>> {
+    static func query(on database: Database, key: String, page: Int, pageSize: Int) async throws -> Page<Joined3<Package, Repository, Version>> {
         try await Joined3<Package, Repository, Version>
             .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,13 @@ 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.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)
 
         guard !page.results.isEmpty else {
             throw Abort(.notFound)
@@ -69,7 +71,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

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
     }
 }

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)
         }

Sources/App/Core/SiteURL.swift

@@ -113,7 +113,7 @@ enum SiteURL: Resourceable, Sendable {
     case blogPost(_ slug: Parameter<String>)
     case buildMonitor
     case builds(_ id: Parameter<UUID>)
-    case collections(_ name: Parameter<String>)
+    case collections(_ key: Parameter<String>)
     case docs(Docs)
     case faq
     case home
@@ -122,7 +122,7 @@ enum SiteURL: Resourceable, Sendable {
     case keywords(_ keyword: Parameter<String>)
     case package(_ owner: Parameter<String>, _ repository: Parameter<String>, PackagePathComponents?)
     case packageCollectionAuthor(_ owner: Parameter<String>)
-    case packageCollectionCustom(_ name: Parameter<String>)
+    case packageCollectionCustom(_ key: Parameter<String>)
     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")

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()
+    }
+}

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,16 +72,26 @@ 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?
         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(\.$url == details.url)
-            .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
+                try await collection.update(on: database)
+            }
             return collection
         } else {
             let collection = CustomCollection(details)
@@ -101,7 +115,20 @@ extension CustomCollection {
     }
 
     var details: Details {
-        .init(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
+        }
     }
 }
 

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

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"),

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)")
                     )
             })

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")