Refactor Rageshake URL and Target configuration. (#4337)

* Move the rageshakeURL configuration from the BugReportService to AppSettings.

* Refactor Target to return a handle when configuring.

This helps with reconfiguration and removes the @MainActor constraint.

UnitTests/Sources/BugReportServiceTests.swift

@@ -12,14 +12,23 @@ import Foundation
 import XCTest
 
 class BugReportServiceTests: XCTestCase {
+    var appSettings: AppSettings!
     var bugReportService: BugReportServiceProtocol!
 
     override func setUpWithError() throws {
+        AppSettings.resetAllSettings()
+        appSettings = AppSettings()
+        appSettings.bugReportRageshakeURL.reset()
+        
         let bugReportServiceMock = BugReportServiceMock()
         bugReportServiceMock.underlyingCrashedLastRun = false
         bugReportServiceMock.submitBugReportProgressListenerReturnValue = .success(SubmitBugReportResponse(reportURL: "https://www.example.com/123"))
         bugReportService = bugReportServiceMock
     }
+    
+    override func tearDown() {
+        appSettings.bugReportRageshakeURL.reset()
+    }
 
     func testInitialStateWithMockService() {
         XCTAssertFalse(bugReportService.crashedLastRun)
@@ -42,7 +51,8 @@ class BugReportServiceTests: XCTestCase {
     }
     
     func testInitialStateWithRealService() throws {
-        let service = BugReportService(rageshakeURL: "https://example.com/submit",
+        let urlPublisher: CurrentValueSubject<RageshakeConfiguration, Never> = .init(.url("https://example.com/submit"))
+        let service = BugReportService(rageshakeURLPublisher: urlPublisher.asCurrentValuePublisher(),
                                        applicationID: "mock_app_id",
                                        sdkGitSHA: "1234",
                                        maxUploadSize: ServiceLocator.shared.settings.bugReportMaxUploadSize,
@@ -52,8 +62,9 @@ class BugReportServiceTests: XCTestCase {
         XCTAssertFalse(service.crashedLastRun)
     }
     
-    func testInitialStateWithRealServiceAndNoURL() throws {
-        let service = BugReportService(rageshakeURL: nil,
+    func testInitialStateWithRealServiceAndDisabled() throws {
+        let urlPublisher: CurrentValueSubject<RageshakeConfiguration, Never> = .init(.disabled)
+        let service = BugReportService(rageshakeURLPublisher: urlPublisher.asCurrentValuePublisher(),
                                        applicationID: "mock_app_id",
                                        sdkGitSHA: "1234",
                                        maxUploadSize: ServiceLocator.shared.settings.bugReportMaxUploadSize,
@@ -64,7 +75,8 @@ class BugReportServiceTests: XCTestCase {
     }
     
     @MainActor func testSubmitBugReportWithRealService() async throws {
-        let service = BugReportService(rageshakeURL: "https://example.com/submit",
+        let urlPublisher: CurrentValueSubject<RageshakeConfiguration, Never> = .init(.url("https://example.com/submit"))
+        let service = BugReportService(rageshakeURLPublisher: urlPublisher.asCurrentValuePublisher(),
                                        applicationID: "mock_app_id",
                                        sdkGitSHA: "1234",
                                        maxUploadSize: ServiceLocator.shared.settings.bugReportMaxUploadSize,
@@ -87,7 +99,12 @@ class BugReportServiceTests: XCTestCase {
     }
     
     @MainActor func testConfigurations() async throws {
-        let service = BugReportService(rageshakeURL: "https://example.com/submit",
+        guard case let .url(initialURL) = appSettings.bugReportRageshakeURL.publisher.value else {
+            XCTFail("Unexpected initial configuration.")
+            return
+        }
+        
+        let service = BugReportService(rageshakeURLPublisher: appSettings.bugReportRageshakeURL.publisher,
                                        applicationID: "mock_app_id",
                                        sdkGitSHA: "1234",
                                        maxUploadSize: ServiceLocator.shared.settings.bugReportMaxUploadSize,
@@ -95,10 +112,10 @@ class BugReportServiceTests: XCTestCase {
                                        appHooks: AppHooks())
         XCTAssertTrue(service.isEnabled)
         
-        service.applyConfiguration(.disabled)
+        appSettings.bugReportRageshakeURL.override(.disabled)
         XCTAssertFalse(service.isEnabled)
         
-        service.applyConfiguration(.url("https://bugs.server.net/submit"))
+        appSettings.bugReportRageshakeURL.override(.url("https://bugs.server.net/submit"))
         XCTAssertTrue(service.isEnabled)
 
         let bugReport = BugReport(userID: "@mock:client.com",
@@ -115,31 +132,12 @@ class BugReportServiceTests: XCTestCase {
         
         XCTAssertEqual(customConfigurationResponse.reportURL, "https://bugs.server.net/123")
         
-        service.applyConfiguration(.default)
+        appSettings.bugReportRageshakeURL.reset()
         XCTAssertTrue(service.isEnabled)
         
         let defaultConfigurationResponse = try await service.submitBugReport(bugReport, progressListener: progressSubject).get()
         
-        XCTAssertEqual(defaultConfigurationResponse.reportURL, "https://example.com/123")
-    }
-    
-    func testDisabledConfigurations() {
-        let service = BugReportService(rageshakeURL: nil,
-                                       applicationID: "mock_app_id",
-                                       sdkGitSHA: "1234",
-                                       maxUploadSize: ServiceLocator.shared.settings.bugReportMaxUploadSize,
-                                       session: .mock,
-                                       appHooks: AppHooks())
-        XCTAssertFalse(service.isEnabled)
-        
-        service.applyConfiguration(.disabled)
-        XCTAssertFalse(service.isEnabled)
-        
-        service.applyConfiguration(.url("https://bugs.server.net/submit"))
-        XCTAssertTrue(service.isEnabled)
-        
-        service.applyConfiguration(.default)
-        XCTAssertFalse(service.isEnabled)
+        XCTAssertEqual(defaultConfigurationResponse.reportURL, initialURL.absoluteString.replacingOccurrences(of: "submit", with: "123"))
     }
     
     func testLogsMaxSize() {

UnitTests/Sources/ConfigurableSettingTests.swift

@@ -0,0 +1,40 @@
+//
+// Copyright 2023, 2024 New Vector Ltd.
+//
+// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+// Please see LICENSE files in the repository root for full details.
+//
+
+import XCTest
+
+@testable import ElementX
+
+class ConfigurableSettingTests: XCTestCase {
+    func testOverrideAndReset() {
+        let setting = ConfigurableSetting(0)
+        XCTAssertEqual(setting.publisher.value, 0)
+        
+        setting.override(1)
+        XCTAssertEqual(setting.publisher.value, 1)
+        
+        setting.override(2)
+        XCTAssertEqual(setting.publisher.value, 2)
+        
+        setting.reset()
+        XCTAssertEqual(setting.publisher.value, 0)
+    }
+    
+    func testOptionalOverride() {
+        let setting: ConfigurableSetting<String?> = .init("Hello")
+        XCTAssertEqual(setting.publisher.value, "Hello")
+        
+        setting.override("World")
+        XCTAssertEqual(setting.publisher.value, "World")
+        
+        setting.override(nil)
+        XCTAssertEqual(setting.publisher.value, nil)
+        
+        setting.reset()
+        XCTAssertEqual(setting.publisher.value, "Hello")
+    }
+}

UnitTests/Sources/LoggingTests.swift

@@ -10,6 +10,8 @@
 import XCTest
 
 class LoggingTests: XCTestCase {
+    static var targetConfiguration: Target.Configuration?
+    
     private enum Constants {
         static let genericFailure = "Test failed"
     }
@@ -22,7 +24,9 @@ class LoggingTests: XCTestCase {
         let target = "tests"
         XCTAssertTrue(Tracing.logFiles.isEmpty)
         
-        await Target.tests.configure(logLevel: .info, traceLogPacks: [], sentryURL: nil)
+        if Self.targetConfiguration == nil {
+            Self.targetConfiguration = Target.tests.configure(logLevel: .info, traceLogPacks: [], sentryURL: nil)
+        }
         
         // There is something weird with Rust logging where the file writing handle doesn't
         // notice that the file it is writing to was deleted, so we can't run these checks
@@ -175,7 +179,9 @@ class LoggingTests: XCTestCase {
                                                               contentType: nil))
         
         // When logging that value
-        await Target.tests.configure(logLevel: .info, traceLogPacks: [], sentryURL: nil)
+        if Self.targetConfiguration == nil {
+            Self.targetConfiguration = Target.tests.configure(logLevel: .info, traceLogPacks: [], sentryURL: nil)
+        }
         
         MXLog.info(textMessage)
         MXLog.info(noticeMessage)