diff --git a/Runtime/Native/Windows/NativeClient.cs b/Runtime/Native/Windows/NativeClient.cs index 35e502d6..4c0d149f 100644 --- a/Runtime/Native/Windows/NativeClient.cs +++ b/Runtime/Native/Windows/NativeClient.cs @@ -333,6 +333,7 @@ private string GetPluginDirectoryPath() const string pluginDir = "Plugins"; return Path.Combine(Application.dataPath, pluginDir); } + /// /// Generate path to Crashpad handler binary /// @@ -367,12 +368,27 @@ internal static void CleanScopedAttributes() { return; } - var attributes = JsonUtility.FromJson(attributesJson); - foreach (var attributeKey in attributes.Keys) + + try + { + var attributes = JsonUtility.FromJson(attributesJson); + if (attributes?.Keys != null) + { + foreach (var attributeKey in attributes.Keys) + { + PlayerPrefs.DeleteKey(string.Format(ScopedAttributesPattern, attributeKey)); + } + } + } + catch (Exception e) { - PlayerPrefs.DeleteKey(string.Format(ScopedAttributesPattern, attributeKey)); + Debug.LogWarning($"Backtrace Failed to parse scoped attributes for cleanup: {e.Message}"); + } + finally + { + PlayerPrefs.DeleteKey(ScopedAttributeListKey); + PlayerPrefs.Save(); } - PlayerPrefs.DeleteKey(ScopedAttributeListKey); } internal static IDictionary GetScopedAttributes() @@ -382,8 +398,25 @@ internal static IDictionary GetScopedAttributes() { return new Dictionary(); } - var result = new Dictionary(); - var attributes = JsonUtility.FromJson(attributesJson); + + var result = new Dictionary(StringComparer.Ordinal); + ScopedAttributesContainer attributes = null; + + try + { + attributes = JsonUtility.FromJson(attributesJson); + } + catch (Exception e) + { + Debug.LogWarning($"Backtrace Failed to parse scoped attributes at read: {e.Message}"); + return result; + } + + if (attributes?.Keys == null) + { + return result; + } + foreach (var attributeKey in attributes.Keys) { var value = PlayerPrefs.GetString(string.Format(ScopedAttributesPattern, attributeKey), string.Empty); @@ -411,7 +444,7 @@ internal static IDictionary GetScopedAttributes() } /// - /// Adds attributes to scoped registry and to native clietn + /// Adds attributes to scoped registry and to native client /// /// attribute key /// attribute value @@ -424,23 +457,103 @@ private void AddAttributes(string key, string value) AddScopedAttribute(key, value); } + /// Load a deduped set of scoped attribute keys from PlayerPrefs. + private HashSet LoadScopedKeys() + { + var keys = new HashSet(StringComparer.Ordinal); + var attributesJson = PlayerPrefs.GetString(ScopedAttributeListKey); + if (HasScopedAttributesEmpty(attributesJson)) + { + try + { + var container = JsonUtility.FromJson(attributesJson); + if (container?.Keys != null && container.Keys.Count > 0) + { + foreach (var k in container.Keys) + { + if (!string.IsNullOrEmpty(k)) + { + keys.Add(k); + } + } + } + } + catch (Exception e) + { + Debug.LogWarning($"Backtrace Failed to parse scoped attributes list. {e.Message}"); + keys.Clear(); + } + } + return keys; + } + + /// Persist a deduped list of keys back to PlayerPrefs. + private void SaveScopedKeys(HashSet keys) + { + var container = new ScopedAttributesContainer + { + Keys = keys.OrderBy(k => k, StringComparer.Ordinal).ToList() + }; + PlayerPrefs.SetString(ScopedAttributeListKey, JsonUtility.ToJson(container)); + PlayerPrefs.Save(); + } + + /// Return the currently stored "value" for a scoped key or "null" if missing. + private string GetScopedValue(string key) + { + var prefsKey = string.Format(ScopedAttributesPattern, key); + return PlayerPrefs.HasKey(prefsKey) ? PlayerPrefs.GetString(prefsKey) : null; + } + + /// Set the stored value for a scoped key. + private void SetScopedValue(string key, string value) + { + PlayerPrefs.SetString(string.Format(ScopedAttributesPattern, key), value ?? string.Empty); + } + /// /// Adds dictionary of attributes to player prefs for windows crashes captured by unity crash handler /// - /// Attributes + /// Attributes internal void AddScopedAttributes(IDictionary attributes) { if (!_configuration.SendUnhandledGameCrashesOnGameStartup) { return; } - var attributesContainer = new ScopedAttributesContainer(); - foreach (var attribute in attributes) + if (attributes == null || attributes.Count == 0) { - attributesContainer.Keys.Add(attribute.Key); - PlayerPrefs.SetString(string.Format(ScopedAttributesPattern, attribute.Key), attribute.Value); + return; } - PlayerPrefs.SetString(ScopedAttributeListKey, JsonUtility.ToJson(attributesContainer)); + + var keys = LoadScopedKeys(); + bool listChanged = false; + + foreach (var kv in attributes) + { + var key = kv.Key; + var value = kv.Value ?? string.Empty; + + // skip when unchanged + var current = GetScopedValue(key); + if (current == null || !string.Equals(current, value, StringComparison.Ordinal)) + { + SetScopedValue(key, value); + } + + // deduplication + if (keys.Add(key)) + { + listChanged = true; + } + } + + if (listChanged) + { + SaveScopedKeys(keys); + } + + PlayerPrefs.Save(); } /// @@ -454,14 +567,29 @@ private void AddScopedAttribute(string key, string value) { return; } - var attributesJson = PlayerPrefs.GetString(ScopedAttributeListKey); - var attributes = HasScopedAttributesEmpty(attributesJson) - ? JsonUtility.FromJson(attributesJson) - : new ScopedAttributesContainer(); + if (string.IsNullOrEmpty(key)) + { + return; + } + + var normalized = value ?? string.Empty; + + // skip when unchanged + var current = GetScopedValue(key); + if (current != null && string.Equals(current, normalized, StringComparison.Ordinal)) + { + return; + } + + SetScopedValue(key, normalized); + + var keys = LoadScopedKeys(); + if (keys.Add(key)) + { + SaveScopedKeys(keys); + } - attributes.Keys.Add(key); - PlayerPrefs.SetString(ScopedAttributeListKey, JsonUtility.ToJson(attributes)); - PlayerPrefs.SetString(string.Format(ScopedAttributesPattern, key), value); + PlayerPrefs.Save(); } private static bool HasScopedAttributesEmpty(string attributesJson) diff --git a/Tests/Runtime/Native/Windows/ScopedNativeAttributesTests.cs b/Tests/Runtime/Native/Windows/ScopedNativeAttributesTests.cs index 1bde3e03..7e7f59c4 100644 --- a/Tests/Runtime/Native/Windows/ScopedNativeAttributesTests.cs +++ b/Tests/Runtime/Native/Windows/ScopedNativeAttributesTests.cs @@ -10,13 +10,19 @@ namespace Backtrace.Unity.Tests.Runtime.Native.Windows { public sealed class ScopedNativeAttributesTests { + // PlayerPrefs key used by the Windows NativeClient to store the scoped list + private const string ScopedKeyList = "backtrace-scoped-attributes"; + private const string ScopedValuePattern = "bt-{0}"; + [TearDown] public void Setup() { CleanLegacyAttributes(); NativeClient.CleanScopedAttributes(); + PlayerPrefs.DeleteAll(); } + [Test] public void FreshStartup_ShouldntIncludeAnyAttributeFromPlayerPrefs_AttributesAreEmpty() { var attributes = NativeClient.GetScopedAttributes(); @@ -24,6 +30,7 @@ public void FreshStartup_ShouldntIncludeAnyAttributeFromPlayerPrefs_AttributesAr Assert.IsEmpty(attributes); } + [Test] public void LegacyAttributesSupport_ShouldIncludeLegacyAttributesWhenScopedAttributesAreNotAvailable_AllLegacyAttributesArePresent() { string testVersion = "0.1.0"; @@ -82,7 +89,6 @@ public void NativeCrashUploadAttributesSetting_ShouldReadPlayerPrefsWithLegacyAt [Test] public void NativeCrashUploadAttributes_ShouldSetScopedAttributeViaNativeClientApi_AttributePresentsInScopedAttributes() { - var configuration = ScriptableObject.CreateInstance(); configuration.SendUnhandledGameCrashesOnGameStartup = true; const string testAttributeKey = "foo-key-bar-baz"; @@ -93,7 +99,6 @@ public void NativeCrashUploadAttributes_ShouldSetScopedAttributeViaNativeClientA var scopedAttributes = NativeClient.GetScopedAttributes(); Assert.AreEqual(scopedAttributes[testAttributeKey], testAttributeValue); - } [Test] @@ -119,6 +124,63 @@ public void NativeCrashAttributesCleanMethod_ShouldCleanAllScopedAttribtues_Scop Assert.IsNotEmpty(attributesBeforeCleanup); } + [Test] + public void ScopedAttributes_ShouldNotDuplicateKeys_OnRepeatedAdds() + { + var configuration = ScriptableObject.CreateInstance(); + configuration.SendUnhandledGameCrashesOnGameStartup = true; + + const string k = "dup-key"; + const string v = "v1"; + + var client = new NativeClient(configuration, null, new Dictionary(), new List()); + + // Add the same key/value multiple times + client.SetAttribute(k, v); + client.SetAttribute(k, v); + client.SetAttribute(k, v); + + // Verify the stored list + var scoped = NativeClient.GetScopedAttributes(); + Assert.IsTrue(scoped.ContainsKey(k)); + Assert.AreEqual(v, scoped[k]); + + // Inspect the JSON list to ensure one occurrence of the key + var json = PlayerPrefs.GetString(ScopedKeyList); + var occurrences = json.Split('"'); + int count = 0; + foreach (var s in occurrences) + { + if (s == k) count++; + } + Assert.AreEqual(1, count, "Key should be stored once in the scoped key list."); + } + + [Test] + public void ScopedAttributes_ShouldSkipWrites_WhenValueUnchanged() + { + var configuration = ScriptableObject.CreateInstance(); + configuration.SendUnhandledGameCrashesOnGameStartup = true; + + const string k = "stable-key"; + const string v = "same-value"; + + var client = new NativeClient(configuration, null, new Dictionary(), new List()); + + // First write + client.SetAttribute(k, v); + var json1 = PlayerPrefs.GetString(ScopedKeyList); + var val1 = PlayerPrefs.GetString(string.Format(ScopedValuePattern, k)); + + // Second write with same value should be a no-op + client.SetAttribute(k, v); + var json2 = PlayerPrefs.GetString(ScopedKeyList); + var val2 = PlayerPrefs.GetString(string.Format(ScopedValuePattern, k)); + + Assert.AreEqual(json1, json2, "Scoped key list JSON should not change when value is unchanged."); + Assert.AreEqual(val1, val2, "Stored value should remain the same."); + } + private void CleanLegacyAttributes() { PlayerPrefs.DeleteKey(NativeClient.VersionKey);