feat: add registerPlugin so a plugin can be added after the client starts

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,26 @@ 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 are collected first, then `Plugin.register` is called, and only then do those hooks begin
+     running. This ordering differs from registration at configuration time, where a plugin's hooks are active before
+     `register` is called: here a plugin's own hooks will not observe flag evaluations or identify calls that its
+     `register` makes. Once this method returns, later evaluations, identify calls, and track calls do run them.
+
+     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) {
+        let pluginHooks = plugin.getHooks(metadata: environmentMetadata)
+        plugin.register(client: self, metadata: environmentMetadata)
+        addHooks(pluginHooks)
+    }
+
     /**
      Tells the SDK to immediately send any currently queued events to LaunchDarkly.
 
@@ -851,17 +896,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 +996,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 +1014,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()

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())

LaunchDarkly/LaunchDarkly/LDClientVariation.swift

@@ -144,6 +144,7 @@ extension LDClient {
     }
 
     private func evaluateWithHooks<D>(flagKey: LDFlagKey, defaultValue: D, methodName: String, featureFlag: FeatureFlag?, evaluation: () -> LDEvaluationDetail<D>) -> LDEvaluationDetail<D> where D: LDValueConvertible, D: Decodable {
+        let hooks = self.hooks
         guard !hooks.isEmpty else {
             return evaluation()
         }

LaunchDarkly/LaunchDarklyTests/LDClientPluginsSpec.swift

@@ -83,6 +83,149 @@ 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 testRegisterPluginDoesNotRunTheRegisteringPluginsOwnHooks() {
+        // 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)
+        }
+
+        testContext.subject.registerPlugin(mockPlugin)
+        XCTAssertEqual(mockPlugin.getCallRecord(), [])
+
+        // They do run for evaluations made once registration has completed.
+        testContext.subject.boolVariation(forKey: "test-flag", defaultValue: false)
+        XCTAssertEqual(mockPlugin.getCallRecord(), ["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] = []