diff --git a/LaunchDarkly/LaunchDarkly/LDClient.swift b/LaunchDarkly/LaunchDarkly/LDClient.swift index 8a9e15c7..add8fe5c 100644 --- a/LaunchDarkly/LaunchDarkly/LDClient.swift +++ b/LaunchDarkly/LaunchDarkly/LDClient.swift @@ -281,10 +281,34 @@ public class LDClient { /// a path an application may take on every redraw of a view. let mobileKeyHash: String let service: DarklyServiceProvider - /// The hooks registered with this client: the configuration's. - /// Constant, so that a series reading it more than once, as an evaluation series does for its before and after - /// stages, runs the same hooks in both, whichever thread the evaluation was made from. - let hooks: [Hook] + /// The hooks registered with this client: the configuration's, then those contributed by plugins. + /// Replaced rather than modified in place, so that a caller of `registerPlugin` on one thread cannot be seen half + /// way by a series running on another. + private var storedHooks: [Hook] + private let hooksLock = NSLock() + + /// A snapshot of the hooks to run. Every series reads this once into a local and works from that, so the hooks a + /// series ends with are the hooks it began with: were it read again mid-series, a hook registered in between would + /// be given an "after" stage for a series whose "before" stage it was never in. + var hooks: [Hook] { + hooksLock.lock() + defer { hooksLock.unlock() } + return storedHooks + } + + /// Describes this client's environment to a plugin. Retained so that a plugin registered later, via + /// `registerPlugin`, is handed the same description as one configured up front. + let environmentMetadata: EnvironmentMetadata + + /// Adds hooks, which the next series to begin will run. A series already under way runs the hooks it began with. + private func addHooks(_ newHooks: [Hook]) { + guard !newHooks.isEmpty else { return } + + hooksLock.lock() + defer { hooksLock.unlock() } + storedHooks.append(contentsOf: newHooks) + } + private(set) var context: LDContext /** @@ -733,6 +757,7 @@ public class LDClient { } private func executeAfterTrackHooks(key: String, data: LDValue?, metricValue: Double?) { + let hooks = self.hooks guard !hooks.isEmpty else { return } @@ -745,6 +770,24 @@ public class LDClient { } } + /** + Registers a single plugin with this client after the client has been configured and started. To register plugins + before the client starts, set `LDConfig.plugins` instead. + + The plugin's hooks begin running before `Plugin.register` is called, as they do for a plugin registered at + configuration time, so a plugin's own hooks observe the flag evaluations and identify calls that its `register` + makes. A plugin whose `register` fails keeps contributing its hooks, again matching configuration time. + + This registers the plugin with this client only. In a multi-environment configuration each environment has its own + client, so registering with every environment means calling this on each of them. + + - parameter plugin: The plugin to register. + */ + public func registerPlugin(_ plugin: Plugin) { + addHooks(plugin.getHooks(metadata: environmentMetadata)) + plugin.register(client: self, metadata: environmentMetadata) + } + /** Tells the SDK to immediately send any currently queued events to LaunchDarkly. @@ -851,17 +894,10 @@ public class LDClient { LDClient.instances?[name] = instance } - let sdkMetadata = SdkMetadata(name: SystemCapabilities.systemName, version: ReportingConsts.sdkVersion) - let environmentMetadata = EnvironmentMetadata( - applicationInfo: instance.environmentReporter.applicationInfo, - sdkMetadata: sdkMetadata, - credential: mobileKey - ) - // now register the client with all the plugins for plugin in config.plugins { do { - plugin.register(client: instance, metadata: environmentMetadata) + plugin.register(client: instance, metadata: instance.environmentMetadata) } catch { os_log("Exception thrown registering plugin %@.", log: config.logger, type: .error, plugin.getMetadata().getName()) } @@ -958,13 +994,8 @@ public class LDClient { /// The hooks the configuration registers, followed by the hooks the plugins contribute. Collected before the init /// identify series opens, so that plugin hooks take part in it. - private static func collectHooks(configuration: LDConfig, environmentReporter: EnvironmentReporting) -> [Hook] { + private static func collectHooks(configuration: LDConfig, metadata: EnvironmentMetadata) -> [Hook] { var hooks = Array(configuration.hooks) - let metadata = EnvironmentMetadata( - applicationInfo: environmentReporter.applicationInfo, - sdkMetadata: SdkMetadata(name: SystemCapabilities.systemName, version: ReportingConsts.sdkVersion), - credential: configuration.mobileKey - ) for plugin in configuration.plugins { do { hooks.append(contentsOf: try plugin.getHooks(metadata: metadata)) @@ -981,8 +1012,14 @@ public class LDClient { self.mobileKeyHash = Util.sha256base64(configuration.mobileKey) self.serviceFactory = serviceFactory environmentReporter = self.serviceFactory.makeEnvironmentReporter(config: configuration) - let hooks = LDClient.collectHooks(configuration: configuration, environmentReporter: environmentReporter) - self.hooks = hooks + let environmentMetadata = EnvironmentMetadata( + applicationInfo: environmentReporter.applicationInfo, + sdkMetadata: SdkMetadata(name: SystemCapabilities.systemName, version: ReportingConsts.sdkVersion), + credential: configuration.mobileKey + ) + self.environmentMetadata = environmentMetadata + let hooks = LDClient.collectHooks(configuration: configuration, metadata: environmentMetadata) + self.storedHooks = hooks flagCache = self.serviceFactory.makeFeatureFlagCache(mobileKey: configuration.mobileKey, maxCachedContexts: configuration.maxCachedContexts) flagStore = self.serviceFactory.makeFlagStore() diff --git a/LaunchDarkly/LaunchDarkly/LDClientIdentifyHook.swift b/LaunchDarkly/LaunchDarkly/LDClientIdentifyHook.swift index 3ccd5acc..1db2f9cc 100644 --- a/LaunchDarkly/LaunchDarkly/LDClientIdentifyHook.swift +++ b/LaunchDarkly/LaunchDarkly/LDClientIdentifyHook.swift @@ -18,11 +18,11 @@ extension LDClient { } internal func executeBeforeIdentifyHooks(context: LDContext) -> IdentifyHookState? { - guard !hooks.isEmpty else { + let hooksSnapshot = self.hooks + guard !hooksSnapshot.isEmpty else { return nil } - let hooksSnapshot = self.hooks let seriesContext = IdentifySeriesContext(context: context, methodName: "identify") let seriesData = hooksSnapshot.map { hook in hook.beforeIdentify(seriesContext: seriesContext, seriesData: EvaluationSeriesData()) diff --git a/LaunchDarkly/LaunchDarkly/LDClientVariation.swift b/LaunchDarkly/LaunchDarkly/LDClientVariation.swift index a91af227..77bd60f8 100644 --- a/LaunchDarkly/LaunchDarkly/LDClientVariation.swift +++ b/LaunchDarkly/LaunchDarkly/LDClientVariation.swift @@ -144,6 +144,7 @@ extension LDClient { } private func evaluateWithHooks(flagKey: LDFlagKey, defaultValue: D, methodName: String, featureFlag: FeatureFlag?, evaluation: () -> LDEvaluationDetail) -> LDEvaluationDetail where D: LDValueConvertible, D: Decodable { + let hooks = self.hooks guard !hooks.isEmpty else { return evaluation() } diff --git a/LaunchDarkly/LaunchDarklyTests/LDClientPluginsSpec.swift b/LaunchDarkly/LaunchDarklyTests/LDClientPluginsSpec.swift index 08bfca00..c56910dc 100644 --- a/LaunchDarkly/LaunchDarklyTests/LDClientPluginsSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/LDClientPluginsSpec.swift @@ -83,6 +83,154 @@ final class LDClientPluginsSpec: XCTestCase { XCTAssertEqual(mockPlugin.getCallRecord()[3], "first after") } + func testRegisterPluginPassesClientAndEnvironmentMetadata() { + var registerCallCount = 0 + var receivedClient: LDClient? + var receivedMetadata: EnvironmentMetadata? + + let mockPlugin = MockPlugin { client, metadata in + registerCallCount += 1 + receivedClient = client + receivedMetadata = metadata + } + + let config = LDConfig(mobileKey: "mobile-key", autoEnvAttributes: .disabled) + var testContext: TestContext! + waitUntil { done in + testContext = TestContext(newConfig: config) + testContext.start(completion: done) + } + + // Nothing happens until the plugin is registered, since it was not in the configuration. + XCTAssertEqual(registerCallCount, 0) + + testContext.subject.registerPlugin(mockPlugin) + + XCTAssertEqual(registerCallCount, 1) + XCTAssertTrue(receivedClient === testContext.subject) + // The same environment description a plugin configured up front would have been given. + XCTAssertEqual(receivedMetadata?.credential, "mobile-key") + XCTAssertEqual(receivedMetadata?.sdkMetadata.name, SystemCapabilities.systemName) + } + + func testRegisterPluginActivatesBundledHooks() { + let mockPlugin = MockPlugin { _, _ in } + + let config = LDConfig(mobileKey: "mobile-key", autoEnvAttributes: .disabled) + var testContext: TestContext! + waitUntil { done in + testContext = TestContext(newConfig: config) + testContext.start(completion: done) + } + + testContext.subject.registerPlugin(mockPlugin) + testContext.subject.boolVariation(forKey: "test-flag", defaultValue: false) + + XCTAssertEqual(mockPlugin.getCallRecord(), ["first before", "second before", "second after", "first after"]) + } + + func testRegisterPluginRunsTheRegisteringPluginsOwnHooks() { + // Evaluates a flag from inside register, so the test can tell whether this plugin's own hooks were live then. + let mockPlugin = MockPlugin { client, _ in + client.boolVariation(forKey: "test-flag", defaultValue: false) + } + + let config = LDConfig(mobileKey: "mobile-key", autoEnvAttributes: .disabled) + var testContext: TestContext! + waitUntil { done in + testContext = TestContext(newConfig: config) + testContext.start(completion: done) + } + + // The hooks are live by the time register runs, as they are for a plugin configured up front, so the + // evaluation register makes passes through them. + testContext.subject.registerPlugin(mockPlugin) + XCTAssertEqual(mockPlugin.getCallRecord(), ["first before", "second before", "second after", "first after"]) + + // And they keep running for evaluations made after registration. + testContext.subject.boolVariation(forKey: "test-flag", defaultValue: false) + XCTAssertEqual(mockPlugin.getCallRecord(), [ + "first before", "second before", "second after", "first after", + "first before", "second before", "second after", "first after" + ]) + } + + func testRegisterPluginHooksRunAfterConfiguredHooks() { + var callRecord: [String] = [] + let record: (String) -> Void = { callRecord.append($0) } + + var config = LDConfig(mobileKey: "mobile-key", autoEnvAttributes: .disabled) + config.hooks = [LDClientPluginsSpec.recordingHook("config", into: record)] + + var testContext: TestContext! + waitUntil { done in + testContext = TestContext(newConfig: config) + testContext.start(completion: done) + } + + let plugin = StubPlugin(hooks: [LDClientPluginsSpec.recordingHook("plugin", into: record)]) + testContext.subject.registerPlugin(plugin) + + testContext.subject.boolVariation(forKey: "test-flag", defaultValue: false) + + // The configured hook was registered first, so it opens the series and, the after stage running in reverse, + // closes it last. + XCTAssertEqual(callRecord, ["config before", "plugin before", "plugin after", "config after"]) + } + + func testRegisterPluginAppliesOnlyToTheClientItIsCalledOn() { + var callRecord: [String] = [] + let record: (String) -> Void = { callRecord.append($0) } + + var config = LDConfig(mobileKey: "primary-mobile-key", autoEnvAttributes: .disabled) + try! config.setSecondaryMobileKeys(["test": "secondary-key-1"]) + + var testContext: TestContext! + waitUntil { done in + testContext = TestContext(newConfig: config) + testContext.start(completion: done) + } + + let plugin = StubPlugin(hooks: [LDClientPluginsSpec.recordingHook("plugin", into: record)]) + testContext.subject.registerPlugin(plugin) + + testContext.subject.boolVariation(forKey: "test-flag", defaultValue: false) + XCTAssertEqual(callRecord, ["plugin before", "plugin after"]) + + // The other environment has its own client, which this plugin was not registered with. + LDClient.get(environment: "test")?.boolVariation(forKey: "test-flag", defaultValue: false) + XCTAssertEqual(callRecord, ["plugin before", "plugin after"]) + } + + private static func recordingHook(_ name: String, into record: @escaping (String) -> Void) -> MockHook { + MockHook( + before: { _, data in record("\(name) before"); return data }, + after: { _, data, _ in record("\(name) after"); return data }) + } + + /// Contributes a fixed set of hooks, and optionally runs a closure when registered. + class StubPlugin: Plugin { + private let hooksToReturn: [Hook] + private let onRegister: (LDClient, EnvironmentMetadata) -> Void + + init(hooks: [Hook], onRegister: @escaping (LDClient, EnvironmentMetadata) -> Void = { _, _ in }) { + self.hooksToReturn = hooks + self.onRegister = onRegister + } + + func getMetadata() -> PluginMetadata { + return PluginMetadata(name: "StubPlugin") + } + + func register(client: LDClient, metadata: EnvironmentMetadata) { + onRegister(client, metadata) + } + + func getHooks(metadata: EnvironmentMetadata) -> [Hook] { + return hooksToReturn + } + } + class MockPlugin: Plugin { private let registerCallback: (LDClient, EnvironmentMetadata) -> Void private var callRecord: [String] = []