From 8ff2c6c8e32de1acc8dda2471e59e04969f619ba Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Fri, 8 Dec 2023 13:18:52 -0500 Subject: [PATCH 1/3] Adding tracking of initialization of plugins --- .../Segment/Analytics/Settings.cs | 15 ++-- Analytics-CSharp/Segment/Analytics/State.cs | 35 ++++++-- .../Segment/Analytics/Timeline.cs | 1 + Tests/Utilities/SettingsTest.cs | 80 +++++++++++++++++++ 4 files changed, 120 insertions(+), 11 deletions(-) create mode 100644 Tests/Utilities/SettingsTest.cs diff --git a/Analytics-CSharp/Segment/Analytics/Settings.cs b/Analytics-CSharp/Segment/Analytics/Settings.cs index f614293..31575c0 100644 --- a/Analytics-CSharp/Segment/Analytics/Settings.cs +++ b/Analytics-CSharp/Segment/Analytics/Settings.cs @@ -15,15 +15,20 @@ public struct Settings public partial class Analytics { - internal void Update(Settings settings, UpdateType type) => Timeline.Apply(plugin => plugin.Update(settings, type)); + internal async void Update(Settings settings) { + System systemState = await Store.CurrentState(); + Timeline.Apply(async plugin => { + UpdateType type = systemState._initializedPlugins.Contains(plugin) ? UpdateType.Refresh : UpdateType.Initial; + plugin.Update(settings, type); + await Store.Dispatch(new System.AddInitializedPluginAction(plugin)); + }); + } - private async Task CheckSettings() + internal async Task CheckSettings() { HTTPClient httpClient = Configuration.HttpClientProvider.CreateHTTPClient(Configuration.WriteKey, cdnHost: Configuration.CdnHost); httpClient.AnalyticsRef = this; System systemState = await Store.CurrentState(); - bool hasSettings = systemState._settings.Integrations != null && systemState._settings.Plan != null; - UpdateType updateType = hasSettings ? UpdateType.Refresh : UpdateType.Initial; await Store.Dispatch(new System.ToggleRunningAction(false)); Settings? settings = null; @@ -41,7 +46,7 @@ await Scope.WithContext(NetworkIODispatcher, async () => settings = systemState._settings; } - Update(settings.Value, updateType); + Update(settings.Value); await Store.Dispatch(new System.ToggleRunningAction(true)); } } diff --git a/Analytics-CSharp/Segment/Analytics/State.cs b/Analytics-CSharp/Segment/Analytics/State.cs index 200234a..8b50254 100644 --- a/Analytics-CSharp/Segment/Analytics/State.cs +++ b/Analytics-CSharp/Segment/Analytics/State.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using global::System; using Segment.Analytics.Utilities; using Segment.Serialization; @@ -19,13 +20,15 @@ internal struct System : IState internal Settings _settings; internal bool _running; internal bool _enable; + internal HashSet _initializedPlugins; - internal System(Configuration configuration, Settings settings, bool running, bool enable) + internal System(Configuration configuration, Settings settings, bool running, bool enable, HashSet initializedPlugins = null) { _configuration = configuration; _settings = settings; _running = running; _enable = enable; + _initializedPlugins = initializedPlugins ?? new HashSet(); } internal static System DefaultState(Configuration configuration, IStorage storage) @@ -42,7 +45,7 @@ internal static System DefaultState(Configuration configuration, IStorage storag settings = configuration.DefaultSettings; } - return new System(configuration, settings, false, true); + return new System(configuration, settings, false, true, null); } internal struct UpdateSettingsAction : IAction @@ -56,7 +59,7 @@ public IState Reduce(IState state) IState result = null; if (state is System systemState) { - result = new System(systemState._configuration, _settings, systemState._running, systemState._enable); + result = new System(systemState._configuration, _settings, systemState._running, systemState._enable, systemState._initializedPlugins); } return result; @@ -74,7 +77,7 @@ public IState Reduce(IState state) IState result = null; if (state is System systemState) { - result = new System(systemState._configuration, systemState._settings, _running, systemState._enable); + result = new System(systemState._configuration, systemState._settings, _running, systemState._enable, systemState._initializedPlugins); } return result; @@ -97,7 +100,7 @@ public IState Reduce(IState state) Settings settings = systemState._settings; settings.Integrations[_key] = true; - result = new System(systemState._configuration, settings, systemState._running, systemState._enable); + result = new System(systemState._configuration, settings, systemState._running, systemState._enable, systemState._initializedPlugins); } return result; @@ -115,7 +118,27 @@ public IState Reduce(IState state) IState result = null; if (state is System systemState) { - result = new System(systemState._configuration, systemState._settings, systemState._running, _enable); + result = new System(systemState._configuration, systemState._settings, systemState._running, _enable, systemState._initializedPlugins); + } + + return result; + } + } + + internal readonly struct AddInitializedPluginAction : IAction + { + private readonly Plugin _plugin; + + public AddInitializedPluginAction(Plugin plugin) => _plugin = plugin; + + public IState Reduce(IState state) + { + IState result = null; + if (state is System systemState) + { + HashSet initializedPlugins = systemState._initializedPlugins; + initializedPlugins.Add(_plugin); + result = new System(systemState._configuration, systemState._settings, systemState._running, systemState._enable, initializedPlugins); } return result; diff --git a/Analytics-CSharp/Segment/Analytics/Timeline.cs b/Analytics-CSharp/Segment/Analytics/Timeline.cs index 8d0b90e..7a6623f 100644 --- a/Analytics-CSharp/Segment/Analytics/Timeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Timeline.cs @@ -163,6 +163,7 @@ internal void Add(Plugin plugin) Settings? settings = await plugin.Analytics.SettingsAsync(); if (settings.HasValue) { + await analytics.Store.Dispatch(new System.AddInitializedPluginAction(plugin)); plugin.Update(settings.Value, UpdateType.Initial); } }); diff --git a/Tests/Utilities/SettingsTest.cs b/Tests/Utilities/SettingsTest.cs new file mode 100644 index 0000000..361fc90 --- /dev/null +++ b/Tests/Utilities/SettingsTest.cs @@ -0,0 +1,80 @@ +using System; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Moq; +using Segment.Analytics; +using Segment.Analytics.Utilities; +using Segment.Serialization; +using Segment.Sovran; +using Tests.Utils; +using Xunit; + +namespace Tests.Utilities +{ + public class SettingsTest + { + private readonly Analytics _analytics; + + private Mock _storage; + + private Settings? _settings; + + public SettingsTest() + { + _settings = JsonUtility.FromJson( + "{\"integrations\":{\"Segment.io\":{\"apiKey\":\"1vNgUqwJeCHmqgI9S1sOm9UHCyfYqbaQ\"}},\"plan\":{},\"edgeFunction\":{}}"); + + var mockHttpClient = new Mock(null, null, null); + mockHttpClient + .Setup(httpClient => httpClient.Settings()) + .ReturnsAsync(_settings); + + _storage = new Mock(); + _storage.Setup(Storage => Storage.RemoveFile("")).Returns(true); + _storage.Setup(Storage => Storage.Read(StorageConstants.Events)).Returns("test,foo"); + + var config = new Configuration( + writeKey: "123", + storageProvider: new MockStorageProvider(_storage), + autoAddSegmentDestination: false, + useSynchronizeDispatcher: true, + httpClientProvider: new MockHttpClientProvider(mockHttpClient) + ); + _analytics = new Analytics(config); + } + + [Fact] + public async Task PluginUpdatesWithInitalOnlyOnce() + { + var plugin = new Mock(); + plugin.Setup(o => o.Key).Returns("mock"); + plugin.Setup(o => o.Analytics).Returns(_analytics); // This would normally be set by Configure + plugin.Setup(o => o.Update(It.IsAny(), It.IsAny())).Callback(() => System.Diagnostics.Debugger.Break()); + + // unlike kotlin, C# should always have settings, so this should run here. + _analytics.Add(plugin.Object); + plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Initial), Times.Once); + plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Refresh), Times.Never); + + // load settings + await _analytics.CheckSettings(); + plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Initial), Times.Once); + plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Refresh), Times.Once); + Segment.Analytics.System system = await _analytics.Store.CurrentState(); + Assert.Contains(plugin.Object, system._initializedPlugins); + + // readd plugin (why would you do this?) + _analytics.Remove(plugin.Object); + _analytics.Add(plugin.Object); + plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Initial), Times.Exactly(2)); + plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Refresh), Times.Once); + + // load settings again + await _analytics.CheckSettings(); + plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Initial), Times.Exactly(2)); + plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Refresh), Times.Exactly(2)); + } + } +} \ No newline at end of file From 9d6ba1821879dff535a7255fb5640aaad2942bba Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Fri, 8 Dec 2023 15:29:47 -0500 Subject: [PATCH 2/3] Using hash instead of object, batching dispatch, fixing init on add logic --- Analytics-CSharp/Segment/Analytics/Settings.cs | 7 +++++-- Analytics-CSharp/Segment/Analytics/State.cs | 14 +++++++------- Analytics-CSharp/Segment/Analytics/Timeline.cs | 7 +++++-- Tests/Utilities/SettingsTest.cs | 5 ++--- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/Analytics-CSharp/Segment/Analytics/Settings.cs b/Analytics-CSharp/Segment/Analytics/Settings.cs index 31575c0..918f6c7 100644 --- a/Analytics-CSharp/Segment/Analytics/Settings.cs +++ b/Analytics-CSharp/Segment/Analytics/Settings.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using global::System.Threading.Tasks; using Segment.Analytics.Utilities; using Segment.Concurrent; @@ -17,11 +18,13 @@ public partial class Analytics { internal async void Update(Settings settings) { System systemState = await Store.CurrentState(); + HashSet initializedPlugins = new HashSet(); Timeline.Apply(async plugin => { - UpdateType type = systemState._initializedPlugins.Contains(plugin) ? UpdateType.Refresh : UpdateType.Initial; + UpdateType type = systemState._initializedPlugins.Contains(plugin.GetHashCode()) ? UpdateType.Refresh : UpdateType.Initial; plugin.Update(settings, type); - await Store.Dispatch(new System.AddInitializedPluginAction(plugin)); + initializedPlugins.Add(plugin.GetHashCode()); }); + await Store.Dispatch(new System.AddInitializedPluginAction(initializedPlugins)); } internal async Task CheckSettings() diff --git a/Analytics-CSharp/Segment/Analytics/State.cs b/Analytics-CSharp/Segment/Analytics/State.cs index 8b50254..49aa430 100644 --- a/Analytics-CSharp/Segment/Analytics/State.cs +++ b/Analytics-CSharp/Segment/Analytics/State.cs @@ -20,15 +20,15 @@ internal struct System : IState internal Settings _settings; internal bool _running; internal bool _enable; - internal HashSet _initializedPlugins; + internal HashSet _initializedPlugins; - internal System(Configuration configuration, Settings settings, bool running, bool enable, HashSet initializedPlugins = null) + internal System(Configuration configuration, Settings settings, bool running, bool enable, HashSet initializedPlugins = null) { _configuration = configuration; _settings = settings; _running = running; _enable = enable; - _initializedPlugins = initializedPlugins ?? new HashSet(); + _initializedPlugins = initializedPlugins ?? new HashSet(); } internal static System DefaultState(Configuration configuration, IStorage storage) @@ -127,17 +127,17 @@ public IState Reduce(IState state) internal readonly struct AddInitializedPluginAction : IAction { - private readonly Plugin _plugin; + private readonly HashSet _pluginHashes; - public AddInitializedPluginAction(Plugin plugin) => _plugin = plugin; + public AddInitializedPluginAction(HashSet pluginHashes) => _pluginHashes = pluginHashes; public IState Reduce(IState state) { IState result = null; if (state is System systemState) { - HashSet initializedPlugins = systemState._initializedPlugins; - initializedPlugins.Add(_plugin); + HashSet initializedPlugins = systemState._initializedPlugins; + initializedPlugins.UnionWith(_pluginHashes); result = new System(systemState._configuration, systemState._settings, systemState._running, systemState._enable, initializedPlugins); } diff --git a/Analytics-CSharp/Segment/Analytics/Timeline.cs b/Analytics-CSharp/Segment/Analytics/Timeline.cs index 7a6623f..9b002b3 100644 --- a/Analytics-CSharp/Segment/Analytics/Timeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Timeline.cs @@ -161,9 +161,12 @@ internal void Add(Plugin plugin) analytics.AnalyticsScope.Launch(analytics.AnalyticsDispatcher, async () => { Settings? settings = await plugin.Analytics.SettingsAsync(); - if (settings.HasValue) + System system = await analytics.Store.CurrentState(); + // Don't initialize unless we have updated settings from the web. + // CheckSettings will initialize everything added before then, so wait until other inits have happened. + if (settings.HasValue && system._initializedPlugins.Count > 0) { - await analytics.Store.Dispatch(new System.AddInitializedPluginAction(plugin)); + await analytics.Store.Dispatch(new System.AddInitializedPluginAction(new HashSet{plugin.GetHashCode()})); plugin.Update(settings.Value, UpdateType.Initial); } }); diff --git a/Tests/Utilities/SettingsTest.cs b/Tests/Utilities/SettingsTest.cs index 361fc90..ebb000c 100644 --- a/Tests/Utilities/SettingsTest.cs +++ b/Tests/Utilities/SettingsTest.cs @@ -51,9 +51,8 @@ public async Task PluginUpdatesWithInitalOnlyOnce() var plugin = new Mock(); plugin.Setup(o => o.Key).Returns("mock"); plugin.Setup(o => o.Analytics).Returns(_analytics); // This would normally be set by Configure - plugin.Setup(o => o.Update(It.IsAny(), It.IsAny())).Callback(() => System.Diagnostics.Debugger.Break()); - // unlike kotlin, C# should always have settings, so this should run here. + // Ideally we'd interrupt init somehow to test adding plugins before, but we don't have a good way _analytics.Add(plugin.Object); plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Initial), Times.Once); plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Refresh), Times.Never); @@ -63,7 +62,7 @@ public async Task PluginUpdatesWithInitalOnlyOnce() plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Initial), Times.Once); plugin.Verify(p => p.Update(It.IsAny(), UpdateType.Refresh), Times.Once); Segment.Analytics.System system = await _analytics.Store.CurrentState(); - Assert.Contains(plugin.Object, system._initializedPlugins); + Assert.Contains(plugin.Object.GetHashCode(), system._initializedPlugins); // readd plugin (why would you do this?) _analytics.Remove(plugin.Object); From 98406d6cf15184e15fdd7e50548105adf736c806 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Mon, 11 Dec 2023 12:53:07 -0500 Subject: [PATCH 3/3] Fixing a stray async that snuck in between revisions --- Analytics-CSharp/Segment/Analytics/Settings.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Analytics-CSharp/Segment/Analytics/Settings.cs b/Analytics-CSharp/Segment/Analytics/Settings.cs index 918f6c7..339b748 100644 --- a/Analytics-CSharp/Segment/Analytics/Settings.cs +++ b/Analytics-CSharp/Segment/Analytics/Settings.cs @@ -19,7 +19,7 @@ public partial class Analytics internal async void Update(Settings settings) { System systemState = await Store.CurrentState(); HashSet initializedPlugins = new HashSet(); - Timeline.Apply(async plugin => { + Timeline.Apply(plugin => { UpdateType type = systemState._initializedPlugins.Contains(plugin.GetHashCode()) ? UpdateType.Refresh : UpdateType.Initial; plugin.Update(settings, type); initializedPlugins.Add(plugin.GetHashCode());