From 7579697db9dac67bce4c299e34d048c450301104 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 15 Oct 2024 15:49:38 +0200 Subject: [PATCH 1/2] Make Package.filter(by: [URL]) case-insensitive and normalise the input URLs --- Sources/App/Models/Package.swift | 9 +++++++-- Tests/AppTests/PackageTests.swift | 19 ++++++++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Sources/App/Models/Package.swift b/Sources/App/Models/Package.swift index 7d9a0e5dd..7b61e863f 100644 --- a/Sources/App/Models/Package.swift +++ b/Sources/App/Models/Package.swift @@ -273,8 +273,13 @@ extension QueryBuilder where Model == Package { } func filter(by urls: some Collection) -> Self { - // TODO: make case-insensitive and canonicalise incoming URLs - filter(\.$url ~~ urls.map(\.absoluteString)) + let urls = urls + .compactMap(\.normalized) + .map { $0.absoluteString.lowercased() } + // Fluent cannot chain `lowercased()` onto `\.$url but the following is essentially + // filter(\.$url.lowercased() ~~ urls) + // by dropping down to SQLKit + return filter(.sql(embed: "LOWER(\(ident: Model.schemaOrAlias).\(ident: Model.path(for: \.$url)[0].description)) IN \(SQLBind.group(urls))")) } } diff --git a/Tests/AppTests/PackageTests.swift b/Tests/AppTests/PackageTests.swift index 36269265b..d1ab1f47a 100644 --- a/Tests/AppTests/PackageTests.swift +++ b/Tests/AppTests/PackageTests.swift @@ -137,11 +137,24 @@ final class PackageTests: AppTestCase { } func test_filter_by_urls() async throws { - for url in ["https://foo.com/1", "https://foo.com/2", "https://foo.com/a", "https://foo.com/A"] { + for url in ["https://foo.com/1.git", "https://foo.com/2.git", "https://foo.com/a.git", "https://foo.com/A.git"] { try await Package(url: url.url).save(on: app.db) } - let res = try await Package.query(on: app.db).filter(by: ["https://foo.com/2", "https://foo.com/a"]).all() - XCTAssertEqual(res.map(\.url), ["https://foo.com/2", "https://foo.com/a"]) + do { // single match + let res = try await Package.query(on: app.db).filter(by: ["https://foo.com/2.git"]).all() + XCTAssertEqual(res.map(\.url), ["https://foo.com/2.git"]) + } + do { // case insensitive match + let res = try await Package.query(on: app.db).filter(by: ["https://foo.com/2.git", "https://foo.com/a.git"]).all() + XCTAssertEqual( + res.map(\.url), + ["https://foo.com/2.git", "https://foo.com/a.git", "https://foo.com/A.git"] + ) + } + do { // input URLs are normalised + let res = try await Package.query(on: app.db).filter(by: ["http://foo.com/2"]).all() + XCTAssertEqual(res.map(\.url), ["https://foo.com/2.git"]) + } } func test_repository() async throws { From 5e4ba0e8bf89f5b49324a1d176092d710e549921 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 15 Oct 2024 16:40:45 +0200 Subject: [PATCH 2/2] Update remaining test expectations --- Sources/App/Core/Extensions/URL+ext.swift | 2 +- Tests/AppTests/CustomCollectionTests.swift | 20 ++++++++-------- Tests/AppTests/ReconcilerTests.swift | 28 +++++++++++----------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/Sources/App/Core/Extensions/URL+ext.swift b/Sources/App/Core/Extensions/URL+ext.swift index 8eddc47ec..73aa18bbb 100644 --- a/Sources/App/Core/Extensions/URL+ext.swift +++ b/Sources/App/Core/Extensions/URL+ext.swift @@ -19,6 +19,6 @@ extension URL { guard var components = URLComponents(url: self, resolvingAgainstBaseURL: false) else { return nil } if components.scheme == "http" { components.scheme = "https" } if !components.path.hasSuffix(".git") { components.path = components.path + ".git" } - return components.url! + return components.url } } diff --git a/Tests/AppTests/CustomCollectionTests.swift b/Tests/AppTests/CustomCollectionTests.swift index 3dbd9c807..373a2b94b 100644 --- a/Tests/AppTests/CustomCollectionTests.swift +++ b/Tests/AppTests/CustomCollectionTests.swift @@ -234,27 +234,27 @@ class CustomCollectionTests: AppTestCase { // 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")) try await collection.save(on: app.db) - try await Package(id: .id1, url: URL("a")).save(on: app.db) - try await Package(id: .id2, url: URL("b")).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) do { // Initial set of URLs // 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 let count = try await CustomCollectionPackage.query(on: app.db).count() 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), ["a"]) + XCTAssertEqual(collection.packages.map(\.url), ["https://github.com/a.git"]) } } do { // Add more URLs // MUT try await collection.reconcile(on: app.db, packageURLs: [ - URL("a"), - URL("b") + URL("https://github.com/a.git"), + URL("https://github.com/b.git") ]) do { // validate @@ -263,8 +263,8 @@ class CustomCollectionTests: AppTestCase { let collection = try await CustomCollection.find(.id0, on: app.db).unwrap() try await collection.$packages.load(on: app.db) XCTAssertEqual(collection.packages.map(\.url).sorted(), [ - "a", - "b" + "https://github.com/a.git", + "https://github.com/b.git" ]) } } @@ -272,7 +272,7 @@ class CustomCollectionTests: AppTestCase { do { // Remove URLs // MUT try await collection.reconcile(on: app.db, packageURLs: [ - URL("b") + URL("https://github.com/b.git") ]) do { // validate @@ -280,7 +280,7 @@ class CustomCollectionTests: AppTestCase { 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), ["b"]) + XCTAssertEqual(collection.packages.map(\.url), ["https://github.com/b.git"]) } } } diff --git a/Tests/AppTests/ReconcilerTests.swift b/Tests/AppTests/ReconcilerTests.swift index 1a7de3bb5..2ea0dfc6a 100644 --- a/Tests/AppTests/ReconcilerTests.swift +++ b/Tests/AppTests/ReconcilerTests.swift @@ -132,12 +132,12 @@ class ReconcilerTests: AppTestCase { func test_reconcileCustomCollections() async throws { // Test single custom collection reconciliation // setup - var fullPackageList = [URL("a"), URL("b"), URL("c")] + var fullPackageList = [URL("https://github.com/a.git"), URL("https://github.com/b.git"), URL("https://github.com/c.git")] for url in fullPackageList { try await Package(url: url).save(on: app.db) } // Initial run try await withDependencies { - $0.packageListRepository.fetchCustomCollection = { @Sendable _, _ in [URL("b")] } + $0.packageListRepository.fetchCustomCollection = { @Sendable _, _ in [URL("https://github.com/b.git")] } } operation: { // MUT try await reconcileCustomCollection(client: app.client, @@ -150,12 +150,12 @@ class ReconcilerTests: AppTestCase { XCTAssertEqual(count, 1) let collection = try await CustomCollection.query(on: app.db).first().unwrap() try await collection.$packages.load(on: app.db) - XCTAssertEqual(collection.packages.map(\.url), ["b"]) + XCTAssertEqual(collection.packages.map(\.url), ["https://github.com/b.git"]) } // Reconcile again with an updated list of packages in the collection try await withDependencies { - $0.packageListRepository.fetchCustomCollection = { @Sendable _, _ in [URL("c")] } + $0.packageListRepository.fetchCustomCollection = { @Sendable _, _ in [URL("https://github.com/c.git")] } } operation: { // MUT try await reconcileCustomCollection(client: app.client, @@ -168,14 +168,14 @@ class ReconcilerTests: AppTestCase { XCTAssertEqual(count, 1) let collection = try await CustomCollection.query(on: app.db).first().unwrap() try await collection.$packages.load(on: app.db) - XCTAssertEqual(collection.packages.map(\.url), ["c"]) + XCTAssertEqual(collection.packages.map(\.url), ["https://github.com/c.git"]) } // Re-run after the single package in the list has been deleted in the full package list - fullPackageList = [URL("a"), URL("b")] - try await Package.query(on: app.db).filter(by: URL("c")).first()?.delete(on: app.db) + fullPackageList = [URL("https://github.com/a.git"), URL("https://github.com/b.git")] + try await Package.query(on: app.db).filter(by: URL("https://github.com/c.git")).first()?.delete(on: app.db) try await withDependencies { - $0.packageListRepository.fetchCustomCollection = { @Sendable _, _ in [URL("c")] } + $0.packageListRepository.fetchCustomCollection = { @Sendable _, _ in [URL("https://github.com/c.git")] } } operation: { // MUT try await reconcileCustomCollection(client: app.client, @@ -195,7 +195,7 @@ class ReconcilerTests: AppTestCase { func test_reconcileCustomCollections_limit() async throws { // Test custom collection reconciliation size limit // setup - let fullPackageList = (1...60).map { URL(string: "\($0)")! } + let fullPackageList = (1...60).map { URL(string: "https://github.com/\($0).git")! } for url in fullPackageList { try await Package(url: url).save(on: app.db) } try await withDependencies { @@ -213,13 +213,13 @@ class ReconcilerTests: AppTestCase { let collection = try await CustomCollection.query(on: app.db).first().unwrap() try await collection.$packages.load(on: app.db) XCTAssertEqual(collection.packages.count, 50) - XCTAssertEqual(collection.packages.first?.url, "1") - XCTAssertEqual(collection.packages.last?.url, "50") + XCTAssertEqual(collection.packages.first?.url, "https://github.com/1.git") + XCTAssertEqual(collection.packages.last?.url, "https://github.com/50.git") } } func test_reconcile() async throws { - let fullPackageList = (1...3).map { URL(string: "\($0)")! } + let fullPackageList = (1...3).map { URL(string: "https://github.com/\($0).git")! } struct TestError: Error { var message: String } try await withDependencies { @@ -227,7 +227,7 @@ class ReconcilerTests: AppTestCase { $0.packageListRepository.fetchPackageDenyList = { @Sendable _ in [] } $0.packageListRepository.fetchCustomCollection = { @Sendable _, url in if url == "collectionURL" { - return [URL("2")] + return [URL("https://github.com/2.git")] } else { throw TestError(message: "collection not found: \(url)") } @@ -249,7 +249,7 @@ class ReconcilerTests: AppTestCase { XCTAssertEqual(collection.name, "List") XCTAssertEqual(collection.url, "collectionURL") try await collection.$packages.load(on: app.db) - XCTAssertEqual(collection.packages.map(\.url), ["2"]) + XCTAssertEqual(collection.packages.map(\.url), ["https://github.com/2.git"]) } }