From cf2d33952f2a31ad1952989b73f8af19bd82873b Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Wed, 18 Dec 2024 18:36:15 -0300 Subject: [PATCH 1/6] WIP: Add loadQuery method --- .../xcschemes/DuckDuckGo.xcscheme | 10 ++ DuckDuckGo/MainViewController.swift | 9 +- LocalPackages/AIChat/Package.swift | 4 + .../AIChat/AIChatWebViewController.swift | 5 + .../Public API/AIChatViewController.swift | 7 ++ .../AIChat/Sources/AIChat/URL+Extension.swift | 35 +++++++ .../AIChat/Tests/URLQueryItemTests.swift | 96 +++++++++++++++++++ 7 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 LocalPackages/AIChat/Sources/AIChat/URL+Extension.swift create mode 100644 LocalPackages/AIChat/Tests/URLQueryItemTests.swift diff --git a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo.xcscheme b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo.xcscheme index 9a439f9d03..7658de6ab4 100644 --- a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo.xcscheme +++ b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo.xcscheme @@ -110,6 +110,16 @@ ReferencedContainer = "container:DuckDuckGo.xcodeproj"> + + + + URL { + guard var urlComponents = URLComponents(url: self, resolvingAgainstBaseURL: false) else { + return self + } + + var queryItems = urlComponents.queryItems ?? [] + queryItems.removeAll { $0.name == queryItem.name } + queryItems.append(queryItem) + + urlComponents.queryItems = queryItems + return urlComponents.url ?? self + } +} diff --git a/LocalPackages/AIChat/Tests/URLQueryItemTests.swift b/LocalPackages/AIChat/Tests/URLQueryItemTests.swift new file mode 100644 index 0000000000..f6a9a548f9 --- /dev/null +++ b/LocalPackages/AIChat/Tests/URLQueryItemTests.swift @@ -0,0 +1,96 @@ +// +// URLQueryItemTests.swift +// DuckDuckGo +// +// Copyright © 2022 DuckDuckGo. All rights reserved. +// +// 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 XCTest +@testable import AIChat + +class URLQueryItemTests: XCTestCase { + + func testAddingQueryItemToEmptyURL() { + let url = URL(string: "https://example.com")! + let queryItem = URLQueryItem(name: "key", value: "value") + let result = url.addingOrReplacingQueryItem(queryItem) + + XCTAssertEqual(result.scheme, "https") + XCTAssertEqual(result.host, "example.com") + XCTAssertEqual(result.queryItemsDictionary, ["key": "value"]) + } + + func testReplacingExistingQueryItem() { + let url = URL(string: "https://example.com?key=oldValue")! + let queryItem = URLQueryItem(name: "key", value: "newValue") + let result = url.addingOrReplacingQueryItem(queryItem) + + XCTAssertEqual(result.scheme, "https") + XCTAssertEqual(result.host, "example.com") + XCTAssertEqual(result.queryItemsDictionary, ["key": "newValue"]) + } + + func testAddingQueryItemToExistingQuery() { + let url = URL(string: "https://example.com?existingKey=existingValue")! + let queryItem = URLQueryItem(name: "newKey", value: "newValue") + let result = url.addingOrReplacingQueryItem(queryItem) + + XCTAssertEqual(result.scheme, "https") + XCTAssertEqual(result.host, "example.com") + XCTAssertEqual(result.queryItemsDictionary, ["existingKey": "existingValue", "newKey": "newValue"]) + } + + func testReplacingOneOfMultipleQueryItems() { + let url = URL(string: "https://example.com?key1=value1&key2=value2")! + let queryItem = URLQueryItem(name: "key1", value: "newValue1") + let result = url.addingOrReplacingQueryItem(queryItem) + + XCTAssertEqual(result.scheme, "https") + XCTAssertEqual(result.host, "example.com") + XCTAssertEqual(result.queryItemsDictionary, ["key1": "newValue1", "key2": "value2"]) + } + + func testAddingQueryItemWithNilValue() { + let url = URL(string: "https://example.com")! + let queryItem = URLQueryItem(name: "key", value: nil) + let result = url.addingOrReplacingQueryItem(queryItem) + + XCTAssertEqual(result.scheme, "https") + XCTAssertEqual(result.host, "example.com") + XCTAssertEqual(result.queryItemsDictionary, ["key": ""]) + } + + func testReplacingQueryItemWithNilValue() { + let url = URL(string: "https://example.com?key=value")! + let queryItem = URLQueryItem(name: "key", value: nil) + let result = url.addingOrReplacingQueryItem(queryItem) + + XCTAssertEqual(result.scheme, "https") + XCTAssertEqual(result.host, "example.com") + XCTAssertEqual(result.queryItemsDictionary, ["key": ""]) + } +} + +extension URL { + var queryItemsDictionary: [String: String] { + var dict = [String: String]() + if let queryItems = URLComponents(url: self, resolvingAgainstBaseURL: false)?.queryItems { + for item in queryItems { + dict[item.name] = item.value ?? "" + } + } + return dict + } +} From 1472616bb528d707f427b70f64f055ab4f10d69f Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Wed, 18 Dec 2024 18:45:28 -0300 Subject: [PATCH 2/6] Validate duck.ai URL --- .../AIChat/AIChatWebViewController.swift | 2 +- .../AIChat/Sources/AIChat/URL+Extension.swift | 13 ++++++ ...temTests.swift => URLExtensionTests.swift} | 44 ++++++++++++++++++- 3 files changed, 56 insertions(+), 3 deletions(-) rename LocalPackages/AIChat/Tests/{URLQueryItemTests.swift => URLExtensionTests.swift} (67%) diff --git a/LocalPackages/AIChat/Sources/AIChat/AIChatWebViewController.swift b/LocalPackages/AIChat/Sources/AIChat/AIChatWebViewController.swift index bb04b03f84..5a15b6546d 100644 --- a/LocalPackages/AIChat/Sources/AIChat/AIChatWebViewController.swift +++ b/LocalPackages/AIChat/Sources/AIChat/AIChatWebViewController.swift @@ -109,7 +109,7 @@ extension AIChatWebViewController: WKNavigationDelegate { func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction) async -> WKNavigationActionPolicy { if let url = navigationAction.request.url { - if url == chatModel.aiChatURL || navigationAction.targetFrame?.isMainFrame == false { + if url.isDuckAIURL || navigationAction.targetFrame?.isMainFrame == false { return .allow } else { delegate?.aiChatWebViewController(self, didRequestToLoad: url) diff --git a/LocalPackages/AIChat/Sources/AIChat/URL+Extension.swift b/LocalPackages/AIChat/Sources/AIChat/URL+Extension.swift index 681695a62b..1e0978f346 100644 --- a/LocalPackages/AIChat/Sources/AIChat/URL+Extension.swift +++ b/LocalPackages/AIChat/Sources/AIChat/URL+Extension.swift @@ -32,4 +32,17 @@ extension URL { urlComponents.queryItems = queryItems return urlComponents.url ?? self } + + var isDuckAIURL: Bool { + guard let host = self.host, host == "duckduckgo.com" else { + return false + } + + guard let urlComponents = URLComponents(url: self, resolvingAgainstBaseURL: false), + let queryItems = urlComponents.queryItems else { + return false + } + + return queryItems.contains { $0.name == "ia" && $0.value == "chat" } + } } diff --git a/LocalPackages/AIChat/Tests/URLQueryItemTests.swift b/LocalPackages/AIChat/Tests/URLExtensionTests.swift similarity index 67% rename from LocalPackages/AIChat/Tests/URLQueryItemTests.swift rename to LocalPackages/AIChat/Tests/URLExtensionTests.swift index f6a9a548f9..fc72d2a759 100644 --- a/LocalPackages/AIChat/Tests/URLQueryItemTests.swift +++ b/LocalPackages/AIChat/Tests/URLExtensionTests.swift @@ -1,5 +1,5 @@ // -// URLQueryItemTests.swift +// URLExtensionTests.swift // DuckDuckGo // // Copyright © 2022 DuckDuckGo. All rights reserved. @@ -20,7 +20,7 @@ import XCTest @testable import AIChat -class URLQueryItemTests: XCTestCase { +class URLExtensionTests: XCTestCase { func testAddingQueryItemToEmptyURL() { let url = URL(string: "https://example.com")! @@ -81,6 +81,46 @@ class URLQueryItemTests: XCTestCase { XCTAssertEqual(result.host, "example.com") XCTAssertEqual(result.queryItemsDictionary, ["key": ""]) } + + func testIsDuckAIURLWithValidURL() { + if let url = URL(string: "https://duckduckgo.com/?ia=chat") { + XCTAssertTrue(url.isDuckAIURL, "The URL should be identified as a DuckDuckGo AI URL.") + } else { + XCTFail("Failed to create URL from string.") + } + } + + func testIsDuckAIURLWithInvalidDomain() { + if let url = URL(string: "https://example.com/?ia=chat") { + XCTAssertFalse(url.isDuckAIURL, "The URL should not be identified as a DuckDuckGo AI URL due to the domain.") + } else { + XCTFail("Failed to create URL from string.") + } + } + + func testIsDuckAIURLWithMissingQueryItem() { + if let url = URL(string: "https://duckduckgo.com/") { + XCTAssertFalse(url.isDuckAIURL, "The URL should not be identified as a DuckDuckGo AI URL due to missing query item.") + } else { + XCTFail("Failed to create URL from string.") + } + } + + func testIsDuckAIURLWithDifferentQueryItem() { + if let url = URL(string: "https://duckduckgo.com/?ia=search") { + XCTAssertFalse(url.isDuckAIURL, "The URL should not be identified as a DuckDuckGo AI URL due to different query item value.") + } else { + XCTFail("Failed to create URL from string.") + } + } + + func testIsDuckAIURLWithAdditionalQueryItems() { + if let url = URL(string: "https://duckduckgo.com/?ia=chat&other=param") { + XCTAssertTrue(url.isDuckAIURL, "The URL should be identified as a DuckDuckGo AI URL even with additional query items.") + } else { + XCTFail("Failed to create URL from string.") + } + } } extension URL { From 9c519a5f64fab56a10e0eac511885953101494b7 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Thu, 19 Dec 2024 14:49:11 -0300 Subject: [PATCH 3/6] Fix issue loading query after memory warning --- .../Sources/AIChat/Public API/AIChatViewController.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/LocalPackages/AIChat/Sources/AIChat/Public API/AIChatViewController.swift b/LocalPackages/AIChat/Sources/AIChat/Public API/AIChatViewController.swift index 79e3a1ad31..6cb555cf1b 100644 --- a/LocalPackages/AIChat/Sources/AIChat/Public API/AIChatViewController.swift +++ b/LocalPackages/AIChat/Sources/AIChat/Public API/AIChatViewController.swift @@ -87,8 +87,12 @@ extension AIChatViewController { // MARK: - Public functions extension AIChatViewController { public func loadQuery(_ query: URLQueryItem) { - webViewController?.loadQuery(query) - } + // Ensure the webViewController is added before loading the query + if webViewController == nil { + addWebViewController() + } + webViewController?.loadQuery(query) + } } // MARK: - Views Setup From 026d3d8db22a2c28b16902e32521219a5cfcc8c9 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Thu, 19 Dec 2024 14:53:32 -0300 Subject: [PATCH 4/6] Linter --- DuckDuckGo/MainViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 8c27ecd4c6..f65abec8e5 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -2091,7 +2091,7 @@ extension MainViewController: OmniBarDelegate { switch accessoryType { case .chat: - let queryItem = currentTab?.url?.getQueryItems()?.filter{ $0.name == "q" }.first + let queryItem = currentTab?.url?.getQueryItems()?.filter { $0.name == "q" }.first openAIChat(queryItem) Pixel.fire(pixel: .openAIChatFromAddressBar) case .share: From 2f24a9e14df07b6a364ceb4054584eaeaf6d4b38 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Fri, 20 Dec 2024 10:52:31 -0300 Subject: [PATCH 5/6] PR feedback --- .../AIChat/Sources/AIChat/URL+Extension.swift | 10 ++++- .../AIChat/Tests/URLExtensionTests.swift | 37 +++++++++++++------ 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/LocalPackages/AIChat/Sources/AIChat/URL+Extension.swift b/LocalPackages/AIChat/Sources/AIChat/URL+Extension.swift index 1e0978f346..b09269d7bb 100644 --- a/LocalPackages/AIChat/Sources/AIChat/URL+Extension.swift +++ b/LocalPackages/AIChat/Sources/AIChat/URL+Extension.swift @@ -20,6 +20,12 @@ import Foundation extension URL { + enum Constants { + static let duckDuckGoHost = "duckduckgo.com" + static let chatQueryName = "ia" + static let chatQueryValue = "chat" + } + func addingOrReplacingQueryItem(_ queryItem: URLQueryItem) -> URL { guard var urlComponents = URLComponents(url: self, resolvingAgainstBaseURL: false) else { return self @@ -34,7 +40,7 @@ extension URL { } var isDuckAIURL: Bool { - guard let host = self.host, host == "duckduckgo.com" else { + guard let host = self.host, host == Constants.duckDuckGoHost else { return false } @@ -43,6 +49,6 @@ extension URL { return false } - return queryItems.contains { $0.name == "ia" && $0.value == "chat" } + return queryItems.contains { $0.name == Constants.chatQueryName && $0.value == Constants.chatQueryValue } } } diff --git a/LocalPackages/AIChat/Tests/URLExtensionTests.swift b/LocalPackages/AIChat/Tests/URLExtensionTests.swift index fc72d2a759..caa5b50236 100644 --- a/LocalPackages/AIChat/Tests/URLExtensionTests.swift +++ b/LocalPackages/AIChat/Tests/URLExtensionTests.swift @@ -20,10 +20,23 @@ import XCTest @testable import AIChat -class URLExtensionTests: XCTestCase { +final class URLExtensionTests: XCTestCase { + private enum TestURLs { + static let exampleDomain = "https://example.com" + static let duckDuckGoDomain = "https://duckduckgo.com" + + static let example = "\(exampleDomain)" + static let exampleWithKeyOldValue = "\(exampleDomain)?key=oldValue" + static let exampleWithExistingQuery = "\(exampleDomain)?existingKey=existingValue" + static let exampleWithMultipleQueryItems = "\(exampleDomain)?key1=value1&key2=value2" + static let duckDuckGoChat = "\(duckDuckGoDomain)/?ia=chat" + static let duckDuckGoWithMissingQuery = "\(duckDuckGoDomain)/" + static let duckDuckGoDifferentQuery = "\(duckDuckGoDomain)/?ia=search" + static let duckDuckGoAdditionalQueryItems = "\(duckDuckGoDomain)/?ia=chat&other=param" + } func testAddingQueryItemToEmptyURL() { - let url = URL(string: "https://example.com")! + let url = URL(string: TestURLs.example)! let queryItem = URLQueryItem(name: "key", value: "value") let result = url.addingOrReplacingQueryItem(queryItem) @@ -33,7 +46,7 @@ class URLExtensionTests: XCTestCase { } func testReplacingExistingQueryItem() { - let url = URL(string: "https://example.com?key=oldValue")! + let url = URL(string: TestURLs.exampleWithKeyOldValue)! let queryItem = URLQueryItem(name: "key", value: "newValue") let result = url.addingOrReplacingQueryItem(queryItem) @@ -43,7 +56,7 @@ class URLExtensionTests: XCTestCase { } func testAddingQueryItemToExistingQuery() { - let url = URL(string: "https://example.com?existingKey=existingValue")! + let url = URL(string: TestURLs.exampleWithExistingQuery)! let queryItem = URLQueryItem(name: "newKey", value: "newValue") let result = url.addingOrReplacingQueryItem(queryItem) @@ -53,7 +66,7 @@ class URLExtensionTests: XCTestCase { } func testReplacingOneOfMultipleQueryItems() { - let url = URL(string: "https://example.com?key1=value1&key2=value2")! + let url = URL(string: TestURLs.exampleWithMultipleQueryItems)! let queryItem = URLQueryItem(name: "key1", value: "newValue1") let result = url.addingOrReplacingQueryItem(queryItem) @@ -63,7 +76,7 @@ class URLExtensionTests: XCTestCase { } func testAddingQueryItemWithNilValue() { - let url = URL(string: "https://example.com")! + let url = URL(string: TestURLs.example)! let queryItem = URLQueryItem(name: "key", value: nil) let result = url.addingOrReplacingQueryItem(queryItem) @@ -73,7 +86,7 @@ class URLExtensionTests: XCTestCase { } func testReplacingQueryItemWithNilValue() { - let url = URL(string: "https://example.com?key=value")! + let url = URL(string: "\(TestURLs.example)?key=value")! let queryItem = URLQueryItem(name: "key", value: nil) let result = url.addingOrReplacingQueryItem(queryItem) @@ -83,7 +96,7 @@ class URLExtensionTests: XCTestCase { } func testIsDuckAIURLWithValidURL() { - if let url = URL(string: "https://duckduckgo.com/?ia=chat") { + if let url = URL(string: TestURLs.duckDuckGoChat) { XCTAssertTrue(url.isDuckAIURL, "The URL should be identified as a DuckDuckGo AI URL.") } else { XCTFail("Failed to create URL from string.") @@ -91,7 +104,7 @@ class URLExtensionTests: XCTestCase { } func testIsDuckAIURLWithInvalidDomain() { - if let url = URL(string: "https://example.com/?ia=chat") { + if let url = URL(string: TestURLs.exampleWithExistingQuery) { XCTAssertFalse(url.isDuckAIURL, "The URL should not be identified as a DuckDuckGo AI URL due to the domain.") } else { XCTFail("Failed to create URL from string.") @@ -99,7 +112,7 @@ class URLExtensionTests: XCTestCase { } func testIsDuckAIURLWithMissingQueryItem() { - if let url = URL(string: "https://duckduckgo.com/") { + if let url = URL(string: TestURLs.duckDuckGoWithMissingQuery) { XCTAssertFalse(url.isDuckAIURL, "The URL should not be identified as a DuckDuckGo AI URL due to missing query item.") } else { XCTFail("Failed to create URL from string.") @@ -107,7 +120,7 @@ class URLExtensionTests: XCTestCase { } func testIsDuckAIURLWithDifferentQueryItem() { - if let url = URL(string: "https://duckduckgo.com/?ia=search") { + if let url = URL(string: TestURLs.duckDuckGoDifferentQuery) { XCTAssertFalse(url.isDuckAIURL, "The URL should not be identified as a DuckDuckGo AI URL due to different query item value.") } else { XCTFail("Failed to create URL from string.") @@ -115,7 +128,7 @@ class URLExtensionTests: XCTestCase { } func testIsDuckAIURLWithAdditionalQueryItems() { - if let url = URL(string: "https://duckduckgo.com/?ia=chat&other=param") { + if let url = URL(string: TestURLs.duckDuckGoAdditionalQueryItems) { XCTAssertTrue(url.isDuckAIURL, "The URL should be identified as a DuckDuckGo AI URL even with additional query items.") } else { XCTFail("Failed to create URL from string.") From ac4e08a07e8b08df66cbb0b082a9d2bca62ed5ae Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Fri, 20 Dec 2024 11:16:21 -0300 Subject: [PATCH 6/6] update runner --- .github/workflows/adhoc.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/adhoc.yml b/.github/workflows/adhoc.yml index 5fededcc1a..ded6413c6c 100644 --- a/.github/workflows/adhoc.yml +++ b/.github/workflows/adhoc.yml @@ -21,7 +21,7 @@ on: jobs: make-adhoc: - runs-on: macos-14-xlarge + runs-on: macos-15 name: Make ad-hoc build steps: