From cc5915683b130e85d9f1f5ff7d9593f4bcd8e8f1 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Thu, 5 Aug 2021 12:00:11 -0300 Subject: [PATCH 01/14] Add scope Observer to Sentry options. --- src/Sentry/SentryOptions.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index c70060324f..0019095ec3 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -33,6 +33,11 @@ public bool IsGlobalModeEnabled set => ScopeStackContainer = value ? new GlobalScopeStackContainer() : new AsyncLocalScopeStackContainer(); } + /// + /// A scope set outside of SentrySDK, if set, the global parameters from the SDK's scope will be sent to the observed scope. + /// + public Scope? ScopeObserver { get; set; } + // Override for tests internal ITransport? Transport { get; set; } From a1cbccd99d7f6612adc7540fddaacbf2b91553ee Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Thu, 5 Aug 2021 15:20:04 -0300 Subject: [PATCH 02/14] Sync done --- src/Sentry/IScopeObserver.cs | 9 +++ src/Sentry/Scope.cs | 40 ++++++++-- src/Sentry/SentryOptions.cs | 2 +- src/Sentry/User.cs | 49 +++++++++++- test/Sentry.Tests/ScopeTests.cs | 131 ++++++++++++++++++++++++++++++++ 5 files changed, 221 insertions(+), 10 deletions(-) create mode 100644 src/Sentry/IScopeObserver.cs diff --git a/src/Sentry/IScopeObserver.cs b/src/Sentry/IScopeObserver.cs new file mode 100644 index 0000000000..d0d8371a2c --- /dev/null +++ b/src/Sentry/IScopeObserver.cs @@ -0,0 +1,9 @@ +namespace Sentry +{ + /// + /// Observer for the sync. of Scopes across SDKs. + /// + public interface IScopeObserver : IEventLike + { + } +} diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index cd69a5c801..7b7a846317 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -99,13 +99,30 @@ public Contexts Contexts set => _contexts = value; } + // Internal for testing. + internal Action UserChanged => () => + { + if (Options.ScopeObserver is { } observer) + { + observer.User = User; + } + }; + private User? _user; /// public User User { - get => _user ??= new User(); - set => _user = value; + get + { + _user ??= new User() { PropertyChanged = UserChanged } ; + return _user; + } + set + { + _user = value; + _user.PropertyChanged = UserChanged; + } } /// @@ -237,16 +254,29 @@ public void AddBreadcrumb(Breadcrumb breadcrumb) } _breadcrumbs.Enqueue(breadcrumb); + Options.ScopeObserver?.AddBreadcrumb(breadcrumb); } /// - public void SetExtra(string key, object? value) => _extra[key] = value; + public void SetExtra(string key, object? value) + { + _extra[key] = value; + Options.ScopeObserver?.SetExtra(key, value); + } /// - public void SetTag(string key, string value) => _tags[key] = value; + public void SetTag(string key, string value) + { + _tags[key] = value; + Options.ScopeObserver?.SetTag(key, value); + } /// - public void UnsetTag(string key) => _tags.TryRemove(key, out _); + public void UnsetTag(string key) + { + _tags.TryRemove(key, out _); + Options.ScopeObserver?.UnsetTag(key); + } /// /// Adds an attachment. diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index 0019095ec3..ef72232d79 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -36,7 +36,7 @@ public bool IsGlobalModeEnabled /// /// A scope set outside of SentrySDK, if set, the global parameters from the SDK's scope will be sent to the observed scope. /// - public Scope? ScopeObserver { get; set; } + public IScopeObserver? ScopeObserver { get; set; } // Override for tests internal ITransport? Transport { get; set; } diff --git a/src/Sentry/User.cs b/src/Sentry/User.cs index a1b0952041..fbee8e10f7 100644 --- a/src/Sentry/User.cs +++ b/src/Sentry/User.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; using System.Text.Json; @@ -11,29 +12,61 @@ namespace Sentry /// public sealed class User : IJsonSerializable { + internal Action? PropertyChanged { get; set; } + + private string? _email; + /// /// The email address of the user. /// /// /// The user's email address. /// - public string? Email { get; set; } + public string? Email + { + get => _email; + set + { + _email = value; + PropertyChanged?.Invoke(); + } + } + private string? _id; /// /// The unique ID of the user. /// /// /// The unique identifier. /// - public string? Id { get; set; } + public string? Id + { + get => _id; + set + { + _id = value; + PropertyChanged?.Invoke(); + } + } + private string? _ipAddress; /// /// The IP of the user. /// /// /// The user's IP address. /// - public string? IpAddress { get; set; } + public string? IpAddress + { + get => _ipAddress; + set + { + _ipAddress = value; + PropertyChanged?.Invoke(); + } + } + + internal string? _username; /// /// The username of the user. @@ -41,7 +74,15 @@ public sealed class User : IJsonSerializable /// /// The user's username. /// - public string? Username { get; set; } + public string? Username + { + get => _username; + set + { + _username = value; + PropertyChanged?.Invoke(); + } + } internal IDictionary? InternalOther { get; private set; } diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index 1cbbd60614..994d4543a4 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -1,6 +1,7 @@ using System; using System.IO; using FluentAssertions; +using NSubstitute; using Sentry.Extensibility; using Sentry.Testing; using Xunit; @@ -279,5 +280,135 @@ public void AddBreadcrumb__AddBreadcrumb_RespectLimits(int initialCount, int max //Assert Assert.Equal(expectedCount, scope.Breadcrumbs.Count); } + + [Theory] + [InlineData("123@123.com", null, null)] + [InlineData(null, "my name", null)] + [InlineData(null, null, "my id")] + public void SetUserEmail_ObserverExist_ObserverUserEmailSet(string email, string username, string id) + { + //Arrange + var observer = NSubstitute.Substitute.For(); + var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + + // Act + if (email != null) + { + scope.User.Email = email; + } + else if (username != null) + { + scope.User.Username = username; + } + else + { + scope.User.Id = id; + + } + + // Assert + Assert.Equal(email, observer.User.Email); + Assert.Equal(id, observer.User.Id); + Assert.Equal(username, observer.User.Username); + } + + [Fact] + public void SetUserEmail_ObserverExistWithUser_ObserverUserEmailChanged() + { + //Arrange + var observer = Substitute.For(); + observer.User = new User() { Email = "4321" }; + var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + + // Act + scope.User.Email = "1234"; + + // Assert + Assert.Equal(scope.User.Email, observer.User.Email); + } + + [Fact] + public void UserChanged_Observernull_Ignored() + { + //Arrange + var scope = new Scope(); + Exception exception = null; + + // Act + try + { + scope.UserChanged.Invoke(); + } + catch (Exception ex) + { + exception = ex; + } + + // Assert + Assert.Null(exception); + } + + [Fact] + public void SetTag_ObserverExist_ObserverSetsTag() + { + //Arrange + var observer = Substitute.For(); + var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var expectedKey = "1234"; + var expectedValue = "5678"; + + // Act + scope.SetTag(expectedKey, expectedValue); + + // Assert + observer.Received(1).SetTag(Arg.Is(expectedKey), Arg.Is(expectedValue)); + } + + [Fact] + public void UnsetTag_ObserverExist_ObserverUnsetsTag() + { + //Arrange + var observer = Substitute.For(); + var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var expectedKey = "1234"; + + // Act + scope.UnsetTag(expectedKey); + + // Assert + observer.Received(1).UnsetTag(Arg.Is(expectedKey)); + } + + [Fact] + public void SetExtra_ObserverExist_ObserverSetsExtra() + { + //Arrange + var observer = Substitute.For(); + var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var expectedKey = "1234"; + var expectedValue = "5678"; + + // Act + scope.SetExtra(expectedKey, expectedValue); + + // Assert + observer.Received(1).SetExtra(Arg.Is(expectedKey), Arg.Is(expectedValue)); + } + + [Fact] + public void AddBreadcrumb_ObserverExist_ObserverAddsBreadcrumb() + { + //Arrange + var observer = Substitute.For(); + var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var breadcrumb = new Breadcrumb(message: "1234"); + + // Act + scope.AddBreadcrumb(breadcrumb); + scope.AddBreadcrumb(breadcrumb); + + // Assert + observer.Received(2).AddBreadcrumb(Arg.Is(breadcrumb)); + } } } From 504f535656b4e4b17a659bffb2b44f98d815d8f2 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Thu, 5 Aug 2021 15:22:52 -0300 Subject: [PATCH 03/14] ctrl k+d --- src/Sentry/Scope.cs | 2 +- src/Sentry/SentryOptions.cs | 2 +- src/Sentry/User.cs | 4 +++- test/Sentry.Tests/ScopeTests.cs | 10 +++++----- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 7b7a846317..c8cca461af 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -115,7 +115,7 @@ public User User { get { - _user ??= new User() { PropertyChanged = UserChanged } ; + _user ??= new User() { PropertyChanged = UserChanged }; return _user; } set diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index ef72232d79..8c043521b5 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -34,7 +34,7 @@ public bool IsGlobalModeEnabled } /// - /// A scope set outside of SentrySDK, if set, the global parameters from the SDK's scope will be sent to the observed scope. + /// A scope set outside of Sentry SDK. If set, the global parameters from the SDK's scope will be sent to the observed scope. /// public IScopeObserver? ScopeObserver { get; set; } diff --git a/src/Sentry/User.cs b/src/Sentry/User.cs index fbee8e10f7..629004657b 100644 --- a/src/Sentry/User.cs +++ b/src/Sentry/User.cs @@ -33,6 +33,7 @@ public string? Email } private string? _id; + /// /// The unique ID of the user. /// @@ -50,6 +51,7 @@ public string? Id } private string? _ipAddress; + /// /// The IP of the user. /// @@ -66,7 +68,7 @@ public string? IpAddress } } - internal string? _username; + private string? _username; /// /// The username of the user. diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index 994d4543a4..8bca32cf84 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -259,11 +259,11 @@ public void ClearAttachments_HasAttachments_EmptyList() [Theory] [InlineData(0, -2, 0)] [InlineData(0, -1, 0)] - [InlineData(0, 0, 0)] - [InlineData(0, 1, 1)] - [InlineData(0, 2, 1)] - [InlineData(1, 2, 2)] - [InlineData(2, 2, 2)] + [InlineData(0, 0, 0)] + [InlineData(0, 1, 1)] + [InlineData(0, 2, 1)] + [InlineData(1, 2, 2)] + [InlineData(2, 2, 2)] public void AddBreadcrumb__AddBreadcrumb_RespectLimits(int initialCount, int maxBreadcrumbs, int expectedCount) { //Arrange From 402e53aa8b99bd54d1fa9d76543f63c3ff9a4e28 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Thu, 5 Aug 2021 15:27:50 -0300 Subject: [PATCH 04/14] smaller user getter and update changelog. --- CHANGELOG.md | 2 ++ src/Sentry/Scope.cs | 6 +----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eaa653fe15..307c5e4c70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ ### Features +- Add Scope observer to SentryOptions ([#1153](https://github.com/getsentry/sentry-dotnet/pull/1153)) + ### Fixes - Set error status to transaction if http has exception and ok status ([#1143](https://github.com/getsentry/sentry-dotnet/pull/1143)) diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index c8cca461af..062317d9c1 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -113,11 +113,7 @@ public Contexts Contexts /// public User User { - get - { - _user ??= new User() { PropertyChanged = UserChanged }; - return _user; - } + get => _user ??= new User() { PropertyChanged = UserChanged }; set { _user = value; From ff837c25777d14aed542487b6e937e7bba71cb7f Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Thu, 5 Aug 2021 15:31:13 -0300 Subject: [PATCH 05/14] add test if user class changes. --- src/Sentry/Scope.cs | 1 + test/Sentry.Tests/ScopeTests.cs | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 062317d9c1..a593cfad3e 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -118,6 +118,7 @@ public User User { _user = value; _user.PropertyChanged = UserChanged; + UserChanged.Invoke(); } } diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index 8bca32cf84..4725e3be19 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -327,6 +327,20 @@ public void SetUserEmail_ObserverExistWithUser_ObserverUserEmailChanged() Assert.Equal(scope.User.Email, observer.User.Email); } + [Fact] + public void SetUser_ObserverExistWith_ObserverUserChanged() + { + //Arrange + var observer = Substitute.For(); + var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + + // Act + scope.User = new User() { Email = "1234" }; + + // Assert + Assert.Equal(scope.User.Email, observer.User.Email); + } + [Fact] public void UserChanged_Observernull_Ignored() { From 699982a81647e04ab9583b6c3e6e3780a60d2ec9 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Thu, 5 Aug 2021 15:33:10 -0300 Subject: [PATCH 06/14] removed line. --- test/Sentry.Tests/ScopeTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index 4725e3be19..c643c0c7ae 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -303,7 +303,6 @@ public void SetUserEmail_ObserverExist_ObserverUserEmailSet(string email, string else { scope.User.Id = id; - } // Assert From 60c5eb2f46687b5e511ce4d7545e8bfe24595e70 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Thu, 5 Aug 2021 15:44:06 -0300 Subject: [PATCH 07/14] fix test --- src/Sentry/Scope.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index a593cfad3e..191c2b8b39 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -117,7 +117,10 @@ public User User set { _user = value; - _user.PropertyChanged = UserChanged; + if (_user != null) + { + _user.PropertyChanged = UserChanged; + } UserChanged.Invoke(); } } From 055994df51c483b6c1acf78221534bc2ae9bcc31 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Thu, 5 Aug 2021 16:18:23 -0300 Subject: [PATCH 08/14] Simplified inferface --- src/Sentry/IScopeObserver.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Sentry/IScopeObserver.cs b/src/Sentry/IScopeObserver.cs index d0d8371a2c..fcd06514b9 100644 --- a/src/Sentry/IScopeObserver.cs +++ b/src/Sentry/IScopeObserver.cs @@ -3,7 +3,14 @@ namespace Sentry /// /// Observer for the sync. of Scopes across SDKs. /// - public interface IScopeObserver : IEventLike + public interface IScopeObserver : IHasBreadcrumbs, IHasTags, IHasExtra { + /// + /// Gets the user information. + /// + /// + /// The user. + /// + User User { get; set; } } } From 213cfb098931b21e7072d9cadec77ef1465c668d Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Fri, 6 Aug 2021 10:27:15 -0300 Subject: [PATCH 09/14] Add EnableScopeSync --- src/Sentry/Scope.cs | 23 ++++-- src/Sentry/SentryOptions.cs | 8 +- test/Sentry.Tests/ScopeTests.cs | 132 +++++++++++++++++++++++--------- 3 files changed, 119 insertions(+), 44 deletions(-) diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 191c2b8b39..1d1aaa2ae8 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -102,7 +102,8 @@ public Contexts Contexts // Internal for testing. internal Action UserChanged => () => { - if (Options.ScopeObserver is { } observer) + if (Options.EnableScopeSync && + Options.ScopeObserver is { } observer) { observer.User = User; } @@ -254,28 +255,40 @@ public void AddBreadcrumb(Breadcrumb breadcrumb) } _breadcrumbs.Enqueue(breadcrumb); - Options.ScopeObserver?.AddBreadcrumb(breadcrumb); + if (Options.EnableScopeSync) + { + Options.ScopeObserver?.AddBreadcrumb(breadcrumb); + } } /// public void SetExtra(string key, object? value) { _extra[key] = value; - Options.ScopeObserver?.SetExtra(key, value); + if (Options.EnableScopeSync) + { + Options.ScopeObserver?.SetExtra(key, value); + } } /// public void SetTag(string key, string value) { _tags[key] = value; - Options.ScopeObserver?.SetTag(key, value); + if (Options.EnableScopeSync) + { + Options.ScopeObserver?.SetTag(key, value); + } } /// public void UnsetTag(string key) { _tags.TryRemove(key, out _); - Options.ScopeObserver?.UnsetTag(key); + if (Options.EnableScopeSync) + { + Options.ScopeObserver?.UnsetTag(key); + } } /// diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index 8c043521b5..563a5775de 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -34,10 +34,16 @@ public bool IsGlobalModeEnabled } /// - /// A scope set outside of Sentry SDK. If set, the global parameters from the SDK's scope will be sent to the observed scope. + /// A scope set outside of Sentry SDK. If set, the global parameters from the SDK's scope will be sent to the observed scope.
+ /// NOTE: EnableScopeSync must be set true for the scope to be synced. /// public IScopeObserver? ScopeObserver { get; set; } + /// + /// If true, the SDK's scope will be synced with the observed scope. + /// + public bool EnableScopeSync { get; set; } + // Override for tests internal ITransport? Transport { get; set; } diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index c643c0c7ae..08139fdfac 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -282,14 +282,24 @@ public void AddBreadcrumb__AddBreadcrumb_RespectLimits(int initialCount, int max } [Theory] - [InlineData("123@123.com", null, null)] - [InlineData(null, "my name", null)] - [InlineData(null, null, "my id")] - public void SetUserEmail_ObserverExist_ObserverUserEmailSet(string email, string username, string id) + [InlineData("123@123.com", null, null, true)] + [InlineData("123@123.com", null, null, false)] + [InlineData(null, "my name", null, true)] + [InlineData(null, "my name", null, false)] + [InlineData(null, null, "my id", true)] + [InlineData(null, null, "my id", false)] + public void SetUserEmail_ObserverExist_ObserverUserEmailSetIfEnabled(string email, string username, string id, bool observerEnable) { //Arrange var observer = NSubstitute.Substitute.For(); - var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var scope = new Scope(new SentryOptions() + { + ScopeObserver = observer, + EnableScopeSync = observerEnable + }); + var expectedEmail = observerEnable ? email : null; + var expectedusername = observerEnable ? username : null; + var expectedid = observerEnable ? id : null; // Act if (email != null) @@ -306,45 +316,63 @@ public void SetUserEmail_ObserverExist_ObserverUserEmailSet(string email, string } // Assert - Assert.Equal(email, observer.User.Email); - Assert.Equal(id, observer.User.Id); - Assert.Equal(username, observer.User.Username); + Assert.Equal(expectedEmail, observer.User?.Email); + Assert.Equal(expectedid, observer.User?.Id); + Assert.Equal(expectedusername, observer.User?.Username); } - [Fact] - public void SetUserEmail_ObserverExistWithUser_ObserverUserEmailChanged() + [Theory] + [InlineData("1234", true)] + [InlineData("1234", false)] + public void SetUserEmail_ObserverExistWithUser_ObserverUserEmailChangedIfEnabled(string email, bool observerEnable) { //Arrange + var observerEmail = "abcd"; var observer = Substitute.For(); - observer.User = new User() { Email = "4321" }; - var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + observer.User = new User() { Email = observerEmail }; + var scope = new Scope(new SentryOptions() + { + ScopeObserver = observer, + EnableScopeSync = observerEnable + }); + var expectedEmail = observerEnable ? email : observerEmail; // Act - scope.User.Email = "1234"; + scope.User.Email = email; // Assert - Assert.Equal(scope.User.Email, observer.User.Email); + Assert.Equal(expectedEmail, observer.User.Email); + } - [Fact] - public void SetUser_ObserverExistWith_ObserverUserChanged() + [Theory] + [InlineData("1234", true)] + [InlineData("1234", false)] + public void SetUser_ObserverExist_ObserverUserChangedIfEnabled(string email, bool observerEnable) { //Arrange var observer = Substitute.For(); - var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var scope = new Scope(new SentryOptions() + { + ScopeObserver = observer, + EnableScopeSync = observerEnable + }); + var expectedEmail = observerEnable ? email : null; // Act - scope.User = new User() { Email = "1234" }; + scope.User = new User() { Email = email }; // Assert - Assert.Equal(scope.User.Email, observer.User.Email); + Assert.Equal(expectedEmail, observer.User?.Email); } - [Fact] - public void UserChanged_Observernull_Ignored() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void UserChanged_Observernull_Ignored(bool observerEnable) { //Arrange - var scope = new Scope(); + var scope = new Scope(new SentryOptions() { EnableScopeSync = observerEnable}); Exception exception = null; // Act @@ -361,67 +389,95 @@ public void UserChanged_Observernull_Ignored() Assert.Null(exception); } - [Fact] - public void SetTag_ObserverExist_ObserverSetsTag() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void SetTag_ObserverExist_ObserverSetsTagIfEnabled(bool observerEnable) { //Arrange var observer = Substitute.For(); - var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var scope = new Scope(new SentryOptions() + { + ScopeObserver = observer, + EnableScopeSync = observerEnable + }); var expectedKey = "1234"; var expectedValue = "5678"; + var expectedCount = observerEnable ? 1 : 0; // Act scope.SetTag(expectedKey, expectedValue); // Assert - observer.Received(1).SetTag(Arg.Is(expectedKey), Arg.Is(expectedValue)); + observer.Received(expectedCount).SetTag(Arg.Is(expectedKey), Arg.Is(expectedValue)); } - [Fact] - public void UnsetTag_ObserverExist_ObserverUnsetsTag() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void UnsetTag_ObserverExist_ObserverUnsetsTagIfEnabled(bool observerEnable) { //Arrange var observer = Substitute.For(); - var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var scope = new Scope(new SentryOptions() + { + ScopeObserver = observer, + EnableScopeSync = observerEnable + }); var expectedKey = "1234"; + var expectedCount = observerEnable ? 1 : 0; // Act scope.UnsetTag(expectedKey); // Assert - observer.Received(1).UnsetTag(Arg.Is(expectedKey)); + observer.Received(expectedCount).UnsetTag(Arg.Is(expectedKey)); } - [Fact] - public void SetExtra_ObserverExist_ObserverSetsExtra() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void SetExtra_ObserverExist_ObserverSetsExtraIfEnabled(bool observerEnable) { //Arrange var observer = Substitute.For(); - var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var scope = new Scope(new SentryOptions() + { + ScopeObserver = observer, + EnableScopeSync = observerEnable + }); var expectedKey = "1234"; var expectedValue = "5678"; + var expectedCount = observerEnable ? 1 : 0; // Act scope.SetExtra(expectedKey, expectedValue); // Assert - observer.Received(1).SetExtra(Arg.Is(expectedKey), Arg.Is(expectedValue)); + observer.Received(expectedCount).SetExtra(Arg.Is(expectedKey), Arg.Is(expectedValue)); } - [Fact] - public void AddBreadcrumb_ObserverExist_ObserverAddsBreadcrumb() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void AddBreadcrumb_ObserverExist_ObserverAddsBreadcrumbIfEnabled(bool observerEnable) { //Arrange var observer = Substitute.For(); - var scope = new Scope(new SentryOptions() { ScopeObserver = observer }); + var scope = new Scope(new SentryOptions() + { + ScopeObserver = observer, + EnableScopeSync = observerEnable + }); var breadcrumb = new Breadcrumb(message: "1234"); + var expectedCount = observerEnable ? 2 : 0; // Act scope.AddBreadcrumb(breadcrumb); scope.AddBreadcrumb(breadcrumb); // Assert - observer.Received(2).AddBreadcrumb(Arg.Is(breadcrumb)); + observer.Received(expectedCount).AddBreadcrumb(Arg.Is(breadcrumb)); } } } From d2d5e35429dfd6955e28c8b2bd89b40714aae239 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Fri, 6 Aug 2021 10:36:12 -0300 Subject: [PATCH 10/14] altered IScopeObserver. --- src/Sentry/IScopeObserver.cs | 40 +++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/Sentry/IScopeObserver.cs b/src/Sentry/IScopeObserver.cs index fcd06514b9..803d80c69d 100644 --- a/src/Sentry/IScopeObserver.cs +++ b/src/Sentry/IScopeObserver.cs @@ -1,10 +1,48 @@ +using System.Collections.Generic; + namespace Sentry { /// /// Observer for the sync. of Scopes across SDKs. /// - public interface IScopeObserver : IHasBreadcrumbs, IHasTags, IHasExtra + public interface IScopeObserver { + /// + /// A trail of events which happened prior to an issue. + /// + /// + IReadOnlyCollection Breadcrumbs { get; } + + /// + /// Arbitrary key-value for this event. + /// + IReadOnlyDictionary Tags { get; } + + /// + /// An arbitrary mapping of additional metadata to store with the event. + /// + IReadOnlyDictionary Extra { get; } + + /// + /// Adds a breadcrumb. + /// + void AddBreadcrumb(Breadcrumb breadcrumb); + + /// + /// Sets an extra. + /// + void SetExtra(string key, object? value); + + /// + /// Sets a tag. + /// + void SetTag(string key, string value); + + /// + /// Removes a tag. + /// + void UnsetTag(string key); + /// /// Gets the user information. /// From 2fb47c11fec25e8aecc7ef00f7d905b5caf263b7 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Fri, 6 Aug 2021 11:02:17 -0300 Subject: [PATCH 11/14] fox text pattern. --- test/Sentry.Tests/ScopeTests.cs | 40 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index 08139fdfac..a59139fb80 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -224,16 +224,16 @@ public void GetSpan_ActiveSpans_ReturnsSpan() [Fact] public void AddAttachment_AddAttachments() { - //Arrange + // Arrange var scope = new Scope(); var attachment = new Attachment(default, default, default, default); var attachment2 = new Attachment(default, default, default, default); - //Act + // Act scope.AddAttachment(attachment); scope.AddAttachment(attachment2); - //Assert + // Assert scope.Attachments.Should().Contain(attachment, "Attachment was not found."); scope.Attachments.Should().Contain(attachment2, "Attachment2 was not found."); } @@ -241,7 +241,7 @@ public void AddAttachment_AddAttachments() [Fact] public void ClearAttachments_HasAttachments_EmptyList() { - //Arrange + // Arrange var scope = new Scope(); for (int i = 0; i < 5; i++) @@ -249,10 +249,10 @@ public void ClearAttachments_HasAttachments_EmptyList() scope.AddAttachment(new MemoryStream(1_000), Guid.NewGuid().ToString()); } - //Act + // Act scope.ClearAttachments(); - //Assert + // Assert scope.Attachments.Should().BeEmpty(); } @@ -266,7 +266,7 @@ public void ClearAttachments_HasAttachments_EmptyList() [InlineData(2, 2, 2)] public void AddBreadcrumb__AddBreadcrumb_RespectLimits(int initialCount, int maxBreadcrumbs, int expectedCount) { - //Arrange + // Arrange var scope = new Scope(new SentryOptions() { MaxBreadcrumbs = maxBreadcrumbs }); for (int i = 0; i < initialCount; i++) @@ -274,10 +274,10 @@ public void AddBreadcrumb__AddBreadcrumb_RespectLimits(int initialCount, int max scope.AddBreadcrumb(new Breadcrumb()); } - //Act + // Act scope.AddBreadcrumb(new Breadcrumb()); - //Assert + // Assert Assert.Equal(expectedCount, scope.Breadcrumbs.Count); } @@ -290,7 +290,7 @@ public void AddBreadcrumb__AddBreadcrumb_RespectLimits(int initialCount, int max [InlineData(null, null, "my id", false)] public void SetUserEmail_ObserverExist_ObserverUserEmailSetIfEnabled(string email, string username, string id, bool observerEnable) { - //Arrange + // Arrange var observer = NSubstitute.Substitute.For(); var scope = new Scope(new SentryOptions() { @@ -316,9 +316,9 @@ public void SetUserEmail_ObserverExist_ObserverUserEmailSetIfEnabled(string emai } // Assert - Assert.Equal(expectedEmail, observer.User?.Email); - Assert.Equal(expectedid, observer.User?.Id); - Assert.Equal(expectedusername, observer.User?.Username); + Assert.Equal(expectedEmail, observer.User?.Email); + Assert.Equal(expectedid, observer.User?.Id); + Assert.Equal(expectedusername, observer.User?.Username); } [Theory] @@ -326,7 +326,7 @@ public void SetUserEmail_ObserverExist_ObserverUserEmailSetIfEnabled(string emai [InlineData("1234", false)] public void SetUserEmail_ObserverExistWithUser_ObserverUserEmailChangedIfEnabled(string email, bool observerEnable) { - //Arrange + // Arrange var observerEmail = "abcd"; var observer = Substitute.For(); observer.User = new User() { Email = observerEmail }; @@ -350,7 +350,7 @@ public void SetUserEmail_ObserverExistWithUser_ObserverUserEmailChangedIfEnabled [InlineData("1234", false)] public void SetUser_ObserverExist_ObserverUserChangedIfEnabled(string email, bool observerEnable) { - //Arrange + // Arrange var observer = Substitute.For(); var scope = new Scope(new SentryOptions() { @@ -371,7 +371,7 @@ public void SetUser_ObserverExist_ObserverUserChangedIfEnabled(string email, boo [InlineData(false)] public void UserChanged_Observernull_Ignored(bool observerEnable) { - //Arrange + // Arrange var scope = new Scope(new SentryOptions() { EnableScopeSync = observerEnable}); Exception exception = null; @@ -394,7 +394,7 @@ public void UserChanged_Observernull_Ignored(bool observerEnable) [InlineData(false)] public void SetTag_ObserverExist_ObserverSetsTagIfEnabled(bool observerEnable) { - //Arrange + // Arrange var observer = Substitute.For(); var scope = new Scope(new SentryOptions() { @@ -417,7 +417,7 @@ public void SetTag_ObserverExist_ObserverSetsTagIfEnabled(bool observerEnable) [InlineData(false)] public void UnsetTag_ObserverExist_ObserverUnsetsTagIfEnabled(bool observerEnable) { - //Arrange + // Arrange var observer = Substitute.For(); var scope = new Scope(new SentryOptions() { @@ -439,7 +439,7 @@ public void UnsetTag_ObserverExist_ObserverUnsetsTagIfEnabled(bool observerEnabl [InlineData(false)] public void SetExtra_ObserverExist_ObserverSetsExtraIfEnabled(bool observerEnable) { - //Arrange + // Arrange var observer = Substitute.For(); var scope = new Scope(new SentryOptions() { @@ -462,7 +462,7 @@ public void SetExtra_ObserverExist_ObserverSetsExtraIfEnabled(bool observerEnabl [InlineData(false)] public void AddBreadcrumb_ObserverExist_ObserverAddsBreadcrumbIfEnabled(bool observerEnable) { - //Arrange + // Arrange var observer = Substitute.For(); var scope = new Scope(new SentryOptions() { From ef1d0055c390b4144fc0b9827d7cc18187dc3be2 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Mon, 23 Aug 2021 11:37:37 -0300 Subject: [PATCH 12/14] Requested changes. --- src/Sentry/IScopeObserver.cs | 23 ++----------- src/Sentry/Scope.cs | 6 ++-- src/Sentry/User.cs | 10 +++--- test/Sentry.Tests/ScopeTests.cs | 57 ++++----------------------------- 4 files changed, 16 insertions(+), 80 deletions(-) diff --git a/src/Sentry/IScopeObserver.cs b/src/Sentry/IScopeObserver.cs index 803d80c69d..5e99abbf31 100644 --- a/src/Sentry/IScopeObserver.cs +++ b/src/Sentry/IScopeObserver.cs @@ -7,22 +7,6 @@ namespace Sentry /// public interface IScopeObserver { - /// - /// A trail of events which happened prior to an issue. - /// - /// - IReadOnlyCollection Breadcrumbs { get; } - - /// - /// Arbitrary key-value for this event. - /// - IReadOnlyDictionary Tags { get; } - - /// - /// An arbitrary mapping of additional metadata to store with the event. - /// - IReadOnlyDictionary Extra { get; } - /// /// Adds a breadcrumb. /// @@ -44,11 +28,8 @@ public interface IScopeObserver void UnsetTag(string key); /// - /// Gets the user information. + /// Sets the user information. /// - /// - /// The user. - /// - User User { get; set; } + void SetUser(User? user); } } diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 1d1aaa2ae8..e01d1f6b0d 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -100,12 +100,12 @@ public Contexts Contexts } // Internal for testing. - internal Action UserChanged => () => + internal Action UserChanged => (user) => { if (Options.EnableScopeSync && Options.ScopeObserver is { } observer) { - observer.User = User; + observer.SetUser(user); } }; @@ -122,7 +122,7 @@ public User User { _user.PropertyChanged = UserChanged; } - UserChanged.Invoke(); + UserChanged.Invoke(_user); } } diff --git a/src/Sentry/User.cs b/src/Sentry/User.cs index 629004657b..d7eb8746ec 100644 --- a/src/Sentry/User.cs +++ b/src/Sentry/User.cs @@ -12,7 +12,7 @@ namespace Sentry /// public sealed class User : IJsonSerializable { - internal Action? PropertyChanged { get; set; } + internal Action? PropertyChanged { get; set; } private string? _email; @@ -28,7 +28,7 @@ public string? Email set { _email = value; - PropertyChanged?.Invoke(); + PropertyChanged?.Invoke(this); } } @@ -46,7 +46,7 @@ public string? Id set { _id = value; - PropertyChanged?.Invoke(); + PropertyChanged?.Invoke(this); } } @@ -64,7 +64,7 @@ public string? IpAddress set { _ipAddress = value; - PropertyChanged?.Invoke(); + PropertyChanged?.Invoke(this); } } @@ -82,7 +82,7 @@ public string? Username set { _username = value; - PropertyChanged?.Invoke(); + PropertyChanged?.Invoke(this); } } diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index a59139fb80..a129ffa2ec 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -288,7 +288,7 @@ public void AddBreadcrumb__AddBreadcrumb_RespectLimits(int initialCount, int max [InlineData(null, "my name", null, false)] [InlineData(null, null, "my id", true)] [InlineData(null, null, "my id", false)] - public void SetUserEmail_ObserverExist_ObserverUserEmailSetIfEnabled(string email, string username, string id, bool observerEnable) + public void SetUser_ObserverExist_ObserverUserInvokedIfEnabled(string email, string username, string id, bool observerEnable) { // Arrange var observer = NSubstitute.Substitute.For(); @@ -300,7 +300,7 @@ public void SetUserEmail_ObserverExist_ObserverUserEmailSetIfEnabled(string emai var expectedEmail = observerEnable ? email : null; var expectedusername = observerEnable ? username : null; var expectedid = observerEnable ? id : null; - + var expectedCount = observerEnable ? 1 : 0; // Act if (email != null) { @@ -316,54 +316,9 @@ public void SetUserEmail_ObserverExist_ObserverUserEmailSetIfEnabled(string emai } // Assert - Assert.Equal(expectedEmail, observer.User?.Email); - Assert.Equal(expectedid, observer.User?.Id); - Assert.Equal(expectedusername, observer.User?.Username); - } - - [Theory] - [InlineData("1234", true)] - [InlineData("1234", false)] - public void SetUserEmail_ObserverExistWithUser_ObserverUserEmailChangedIfEnabled(string email, bool observerEnable) - { - // Arrange - var observerEmail = "abcd"; - var observer = Substitute.For(); - observer.User = new User() { Email = observerEmail }; - var scope = new Scope(new SentryOptions() - { - ScopeObserver = observer, - EnableScopeSync = observerEnable - }); - var expectedEmail = observerEnable ? email : observerEmail; - - // Act - scope.User.Email = email; - - // Assert - Assert.Equal(expectedEmail, observer.User.Email); - - } - - [Theory] - [InlineData("1234", true)] - [InlineData("1234", false)] - public void SetUser_ObserverExist_ObserverUserChangedIfEnabled(string email, bool observerEnable) - { - // Arrange - var observer = Substitute.For(); - var scope = new Scope(new SentryOptions() - { - ScopeObserver = observer, - EnableScopeSync = observerEnable - }); - var expectedEmail = observerEnable ? email : null; - - // Act - scope.User = new User() { Email = email }; - - // Assert - Assert.Equal(expectedEmail, observer.User?.Email); + observer.Received(expectedCount).SetUser(Arg.Is((user)=> user.Email == expectedEmail)); + observer.Received(expectedCount).SetUser(Arg.Is((user) => user.Id == expectedid)); + observer.Received(expectedCount).SetUser(Arg.Is((user) => user.Username == expectedusername)); } [Theory] @@ -378,7 +333,7 @@ public void UserChanged_Observernull_Ignored(bool observerEnable) // Act try { - scope.UserChanged.Invoke(); + scope.UserChanged.Invoke(null); } catch (Exception ex) { From 65784f661f2925727f033ab9163be03f9ed81be7 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Mon, 23 Aug 2021 15:00:03 -0300 Subject: [PATCH 13/14] minor naming changes. --- src/Sentry/Scope.cs | 6 +++--- test/Sentry.Tests/ScopeTests.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index e01d1f6b0d..202627422e 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -114,13 +114,13 @@ public Contexts Contexts /// public User User { - get => _user ??= new User() { PropertyChanged = UserChanged }; + get => _user ??= new User { PropertyChanged = UserChanged }; set { _user = value; - if (_user != null) + if (_user is not null) { - _user.PropertyChanged = UserChanged; + user.PropertyChanged = UserChanged; } UserChanged.Invoke(_user); } diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index a129ffa2ec..daec943da2 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -324,7 +324,7 @@ public void SetUser_ObserverExist_ObserverUserInvokedIfEnabled(string email, str [Theory] [InlineData(true)] [InlineData(false)] - public void UserChanged_Observernull_Ignored(bool observerEnable) + public void UserChanged_ObserverNull_Ignored(bool observerEnable) { // Arrange var scope = new Scope(new SentryOptions() { EnableScopeSync = observerEnable}); From fe5b40e2aef54c134ee4e0d713a21110a822a320 Mon Sep 17 00:00:00 2001 From: Lucas Zimerman Fraulob Date: Mon, 23 Aug 2021 15:06:10 -0300 Subject: [PATCH 14/14] fix typo. --- src/Sentry/Scope.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 202627422e..bc31b8db66 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -120,7 +120,7 @@ public User User _user = value; if (_user is not null) { - user.PropertyChanged = UserChanged; + _user.PropertyChanged = UserChanged; } UserChanged.Invoke(_user); }