Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 57 additions & 20 deletions LaunchDarkly/LaunchDarkly/LDClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defer is not always without cost. Do you need defer here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LDClient is the class and defer would be negligible compare to lock/unlock itself

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defer is not always without cost. Do you need defer here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LDClient is the class and defer would be negligible compare to lock/unlock itself

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defer is considered good pattern for locking

storedHooks.append(contentsOf: newHooks)
}

private(set) var context: LDContext

/**
Expand Down Expand Up @@ -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
}
Expand All @@ -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.

Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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))
Expand All @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions LaunchDarkly/LaunchDarkly/LDClientIdentifyHook.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
1 change: 1 addition & 0 deletions LaunchDarkly/LaunchDarkly/LDClientVariation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
148 changes: 148 additions & 0 deletions LaunchDarkly/LaunchDarklyTests/LDClientPluginsSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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] = []
Expand Down
Loading