From c97be22e87e32ca4c1e69bd2eeb53c8e6631efe7 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 4 Jul 2023 08:56:04 +0200 Subject: [PATCH 1/4] Add test_productTypeFilter_macro --- Tests/AppTests/SearchTests.swift | 52 ++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/Tests/AppTests/SearchTests.swift b/Tests/AppTests/SearchTests.swift index 426490946..c0da49702 100644 --- a/Tests/AppTests/SearchTests.swift +++ b/Tests/AppTests/SearchTests.swift @@ -1427,6 +1427,58 @@ class SearchTests: AppTestCase { } } + func test_productTypeFilter_macro() async throws { + // setup + do { + let p1 = Package.init(id: .id0, url: "1".url) + try await p1.save(on: app.db) + try await Repository(package: p1, + defaultBranch: "main", + name: "1", + owner: "foo", + stars: 1, + summary: "test package").save(on: app.db) + let v = try Version(package: p1) + try await v.save(on: app.db) + try await Target(version: v, name: "t1", type: .regular).save(on: app.db) + } + do { + let p2 = Package.init(id: .id1, url: "2".url) + try await p2.save(on: app.db) + try await Repository(package: p2, + defaultBranch: "main", + name: "2", + owner: "foo", + summary: "test package").save(on: app.db) + let v = try Version(package: p2) + try await v.save(on: app.db) + try await Target(version: v, name: "t2", type: .macro).save(on: app.db) + } + try await Search.refresh(on: app.db).get() + + do { + // MUT + let res = try await Search.fetch(app.db, ["test", "product:macro"], page: 1, pageSize: 20).get() + + // validate + XCTAssertEqual(res.results.count, 1) + XCTAssertEqual( + res.results.compactMap(\.packageResult?.repositoryName), ["2"] + ) + } + + do { + // MUT + let res = try await Search.fetch(app.db, ["test"], page: 1, pageSize: 20).get() + + // validate + XCTAssertEqual(res.results.count, 2) + XCTAssertEqual( + res.results.compactMap(\.packageResult?.repositoryName), ["1", "2"] + ) + } + } + func test_SearchFilter_error() throws { // Test error handling in case of an invalid filter // Setup From 62532136667be34cdd2bec7126bf8d79fdd06f67 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 4 Jul 2023 09:06:42 +0200 Subject: [PATCH 2/4] Extend ProductTypeSearchFilter with macro --- .../Filters/ProductTypeSearchFilter.swift | 3 +++ Tests/AppTests/SearchFilterTests.swift | 20 ++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Sources/App/Core/SearchFilter/Filters/ProductTypeSearchFilter.swift b/Sources/App/Core/SearchFilter/Filters/ProductTypeSearchFilter.swift index c8d670c1c..e3dd1ef2c 100644 --- a/Sources/App/Core/SearchFilter/Filters/ProductTypeSearchFilter.swift +++ b/Sources/App/Core/SearchFilter/Filters/ProductTypeSearchFilter.swift @@ -65,6 +65,7 @@ extension ProductTypeSearchFilter { enum ProductType: String, Codable, CaseIterable { case executable case library + case macro case plugin var displayDescription: String { @@ -73,6 +74,8 @@ extension ProductTypeSearchFilter { return "Executable" case .library: return "Library" + case .macro: + return "Macro" case .plugin: return "Plugin" } diff --git a/Tests/AppTests/SearchFilterTests.swift b/Tests/AppTests/SearchFilterTests.swift index dd79b8816..c77083b3f 100644 --- a/Tests/AppTests/SearchFilterTests.swift +++ b/Tests/AppTests/SearchFilterTests.swift @@ -431,11 +431,29 @@ class SearchFilterTests: AppTestCase { XCTAssertEqual(binds(filter.rightHandSide), ["{executable}"]) } + func test_productTypeFilter_macro() throws { + // Test "virtual" macro product filter + let filter = try ProductTypeSearchFilter(expression: .init(operator: .is, value: "macro")) + XCTAssertEqual(filter.key, .productType) + XCTAssertEqual(filter.predicate, .init(operator: .contains, + bindableValue: .value("macro"), + displayValue: "Macro")) + + // test view representation + XCTAssertEqual(filter.viewModel.description, "Package products contain a Macro") + + // test sql representation + XCTAssertEqual(renderSQL(filter.leftHandSide), #""product_types""#) + XCTAssertEqual(renderSQL(filter.sqlOperator), "@>") + XCTAssertEqual(binds(filter.rightHandSide), ["{macro}"]) + } + func test_productTypeFilter_spelling() throws { let expectedDisplayValues = [ ProductTypeSearchFilter.ProductType.executable: "Package products contain an Executable", ProductTypeSearchFilter.ProductType.plugin: "Package products contain a Plugin", - ProductTypeSearchFilter.ProductType.library: "Package products contain a Library" + ProductTypeSearchFilter.ProductType.library: "Package products contain a Library", + ProductTypeSearchFilter.ProductType.macro: "Package products contain a Macro" ] for type in ProductTypeSearchFilter.ProductType.allCases { From 155be1ece1a45cfbc8e7516071bfde7480236963 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 4 Jul 2023 13:02:22 +0200 Subject: [PATCH 3/4] Add "virtual" macro products to search view --- .../066/UpdateSearchAddMacroProductType.swift | 104 ++++++++++++++++++ Sources/App/configure.swift | 3 + Tests/AppTests/QueryPerformanceTests.swift | 12 +- 3 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 Sources/App/Migrations/066/UpdateSearchAddMacroProductType.swift diff --git a/Sources/App/Migrations/066/UpdateSearchAddMacroProductType.swift b/Sources/App/Migrations/066/UpdateSearchAddMacroProductType.swift new file mode 100644 index 000000000..760c6ec91 --- /dev/null +++ b/Sources/App/Migrations/066/UpdateSearchAddMacroProductType.swift @@ -0,0 +1,104 @@ +// 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 +import SQLKit + + +struct UpdateSearchAddMacroProductType: AsyncMigration { + let dropSQL: SQLQueryString = "DROP MATERIALIZED VIEW search" + + func prepare(on database: Database) async throws { + guard let db = database as? SQLDatabase else { + fatalError("Database must be an SQLDatabase ('as? SQLDatabase' must succeed)") + } + + // Create an index on targets.version_id - this speeds up search view creation + // dramatically. + try await db.raw("CREATE INDEX idx_targets_version_id ON targets (version_id)") + .run() + + // ** IMPORTANT ** + // When updating the query underlying the materialized view, make sure to also + // update the matching performance test in QueryPerformanceTests.test_Search_refresh! + try await db.raw(dropSQL).run() + try await db.raw(""" + -- v11 + CREATE MATERIALIZED VIEW search AS + SELECT + p.id AS package_id, + p.platform_compatibility, + p.score, + r.keywords, + r.last_commit_date, + r.license, + r.name AS repo_name, + r.owner AS repo_owner, + r.stars, + r.last_activity_at, + r.summary, + v.package_name, + ARRAY_LENGTH(doc_archives, 1) >= 1 AS has_docs, + ARRAY( + SELECT DISTINCT JSONB_OBJECT_KEYS(type) FROM products WHERE products.version_id = v.id + UNION + SELECT * FROM ( + SELECT DISTINCT JSONB_OBJECT_KEYS(type) AS "type" FROM targets + WHERE targets.version_id = v.id) AS macro_targets + WHERE type = 'macro' + ) AS product_types, + ARRAY(SELECT DISTINCT name FROM products WHERE products.version_id = v.id) AS product_names, + TO_TSVECTOR(CONCAT_WS(' ', COALESCE(v.package_name, ''), r.name, COALESCE(r.summary, ''), ARRAY_TO_STRING(r.keywords, ' '))) AS tsvector + FROM packages p + JOIN repositories r ON r.package_id = p.id + JOIN versions v ON v.package_id = p.id + WHERE v.reference ->> 'branch' = r.default_branch + """).run() + } + + func revert(on database: Database) async throws { + guard let db = database as? SQLDatabase else { + fatalError("Database must be an SQLDatabase ('as? SQLDatabase' must succeed)") + } + + try await db.raw(dropSQL).run() + try await db.raw(""" + -- v10 + CREATE MATERIALIZED VIEW search AS + SELECT + p.id AS package_id, + p.platform_compatibility, + p.score, + r.keywords, + r.last_commit_date, + r.license, + r.name AS repo_name, + r.owner AS repo_owner, + r.stars, + r.last_activity_at, + r.summary, + v.package_name, + ARRAY_LENGTH(doc_archives, 1) >= 1 AS has_docs, + ARRAY(SELECT DISTINCT JSONB_OBJECT_KEYS(type) FROM products WHERE products.version_id = v.id) AS product_types, + ARRAY(SELECT DISTINCT name FROM products WHERE products.version_id = v.id) AS product_names, + TO_TSVECTOR(CONCAT_WS(' ', COALESCE(v.package_name, ''), r.name, COALESCE(r.summary, ''), ARRAY_TO_STRING(r.keywords, ' '))) AS tsvector + FROM packages p + JOIN repositories r ON r.package_id = p.id + JOIN versions v ON v.package_id = p.id + WHERE v.reference ->> 'branch' = r.default_branch + """).run() + + try await db.raw("DROP INDEX idx_targets_version_id").run() + } +} diff --git a/Sources/App/configure.swift b/Sources/App/configure.swift index b6c0c4426..0ceaaa46d 100644 --- a/Sources/App/configure.swift +++ b/Sources/App/configure.swift @@ -287,6 +287,9 @@ public func configure(_ app: Application) throws -> String { do { // Migration 065 - add linkable_paths_count to doc_uploads app.migrations.add(UpdateDocUploadAddLinkablePathsCount()) } + do { // Migration 066 - add virtual macro product type to search view + app.migrations.add(UpdateSearchAddMacroProductType()) + } app.commands.use(Analyze.Command(), as: "analyze") app.commands.use(CreateRestfileCommand(), as: "create-restfile") diff --git a/Tests/AppTests/QueryPerformanceTests.swift b/Tests/AppTests/QueryPerformanceTests.swift index 79cc045b7..837028598 100644 --- a/Tests/AppTests/QueryPerformanceTests.swift +++ b/Tests/AppTests/QueryPerformanceTests.swift @@ -115,6 +115,7 @@ class QueryPerformanceTests: XCTestCase { } func test_12_Search_refresh() async throws { + throw XCTSkip("run only after schema change with new index has been deployed") // We can't "explain analyze" the refresh itself so we need to measure the underlying // query. // Unfortunately, this means it'll need to be kept in sync when updating the search @@ -124,7 +125,7 @@ class QueryPerformanceTests: XCTestCase { return } let query = db.raw(""" - -- v10 + -- v11 SELECT p.id AS package_id, p.platform_compatibility, @@ -139,7 +140,14 @@ class QueryPerformanceTests: XCTestCase { r.summary, v.package_name, array_length(doc_archives, 1) >= 1 AS has_docs, - ARRAY(SELECT DISTINCT JSONB_OBJECT_KEYS(type) FROM products WHERE products.version_id = v.id) AS product_types, + ARRAY( + SELECT DISTINCT JSONB_OBJECT_KEYS(type) FROM products WHERE products.version_id = v.id + UNION + SELECT * FROM ( + SELECT DISTINCT JSONB_OBJECT_KEYS(type) AS "type" FROM targets + WHERE targets.version_id = v.id) AS macro_targets + WHERE type = 'macro' + ) AS product_types, ARRAY(SELECT DISTINCT name FROM products WHERE products.version_id = v.id) AS product_names, TO_TSVECTOR(CONCAT_WS(' ', COALESCE(v.package_name, ''), r.name, COALESCE(r.summary, ''), ARRAY_TO_STRING(r.keywords, ' '))) AS tsvector FROM packages p From 814307bc51d5ebceeb740042aed180a50c36cea0 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Tue, 4 Jul 2023 13:38:30 +0200 Subject: [PATCH 4/4] Update search performance figures --- Tests/AppTests/QueryPerformanceTests.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/AppTests/QueryPerformanceTests.swift b/Tests/AppTests/QueryPerformanceTests.swift index 837028598..d6ff4e505 100644 --- a/Tests/AppTests/QueryPerformanceTests.swift +++ b/Tests/AppTests/QueryPerformanceTests.swift @@ -115,7 +115,6 @@ class QueryPerformanceTests: XCTestCase { } func test_12_Search_refresh() async throws { - throw XCTSkip("run only after schema change with new index has been deployed") // We can't "explain analyze" the refresh itself so we need to measure the underlying // query. // Unfortunately, this means it'll need to be kept in sync when updating the search @@ -155,7 +154,7 @@ class QueryPerformanceTests: XCTestCase { JOIN versions v ON v.package_id = p.id WHERE v.reference ->> 'branch' = r.default_branch """) - try await assertQueryPerformance(query, expectedCost: 31_300, variation: 500) + try await assertQueryPerformance(query, expectedCost: 55_000, variation: 500) } }