diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureReposBindingManagerTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureReposBindingManagerTests.cs index 2c506ae62..ec60d8f70 100644 --- a/src/shared/Microsoft.AzureRepos.Tests/AzureReposBindingManagerTests.cs +++ b/src/shared/Microsoft.AzureRepos.Tests/AzureReposBindingManagerTests.cs @@ -628,6 +628,93 @@ public void AzureReposBindingManager_SignIn_OtherGlobalOtherLocal_BindsLocal() Assert.Equal(user2, actualGlobalUser); } + // Idempotency: SignIn when state is already correct should not write to git config + + [Fact] + public void AzureReposBindingManager_SignIn_SameGlobalNoLocal_NoConfigWrites() + { + // Steady-state: global already bound to signing-in user, no local override. + // This is the common case on every 'git fetch' after the first sign-in. + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] {user1}; + + manager.SignIn(orgName, user1); + + Assert.Equal(0, git.Configuration.SetCallCount); + Assert.Equal(0, git.Configuration.UnsetCallCount); + } + + [Fact] + public void AzureReposBindingManager_SignIn_OtherGlobalSameLocal_NoConfigWrites() + { + // Steady-state: a different user holds the global binding, and local is already + // bound to the signing-in user. No change needed. + const string orgName = "org"; + const string user1 = "user1"; + const string user2 = "user2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] {user2}; + git.Configuration.Local[CreateKey(orgName)] = new[] {user1}; + + manager.SignIn(orgName, user1); + + Assert.Equal(0, git.Configuration.SetCallCount); + Assert.Equal(0, git.Configuration.UnsetCallCount); + } + + [Fact] + public void AzureReposBindingManager_SignIn_SameGlobalSameLocal_OnlyUnbindsLocal() + { + // Global already matches, local redundantly mirrors it. + // Only the local unset is needed; re-writing the global value is wasteful. + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] {user1}; + git.Configuration.Local[CreateKey(orgName)] = new[] {user1}; + + manager.SignIn(orgName, user1); + + Assert.Equal(0, git.Configuration.SetCallCount); + Assert.Equal(1, git.Configuration.UnsetCallCount); + } + + [Fact] + public void AzureReposBindingManager_SignIn_SameGlobalOtherLocal_OnlyUnbindsLocal() + { + // Global already matches, local has a different user that needs removing. + // Only the local unset is needed; re-writing the global value is wasteful. + const string orgName = "org"; + const string user1 = "user1"; + const string user2 = "user2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] {user1}; + git.Configuration.Local[CreateKey(orgName)] = new[] {user2}; + + manager.SignIn(orgName, user1); + + Assert.Equal(0, git.Configuration.SetCallCount); + Assert.Equal(1, git.Configuration.UnsetCallCount); + } + #endregion #region SignOut diff --git a/src/shared/Microsoft.AzureRepos/AzureReposBindingManager.cs b/src/shared/Microsoft.AzureRepos/AzureReposBindingManager.cs index 7e26590bd..2ae7fd11a 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposBindingManager.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposBindingManager.cs @@ -283,12 +283,24 @@ public static void SignIn(this IAzureReposBindingManager bindingManager, string if (existingBinding?.GlobalUserName != null && !StringComparer.OrdinalIgnoreCase.Equals(existingBinding.GlobalUserName, userName)) { - bindingManager.Bind(orgName, userName, local: true); + // Global is bound to a different user (B); bind this user locally (-> B | A). + // Skip the write if local is already correct. + if (!StringComparer.OrdinalIgnoreCase.Equals(existingBinding.LocalUserName, userName)) + { + bindingManager.Bind(orgName, userName, local: true); + } } else { - bindingManager.Bind(orgName, userName, local: false); - bindingManager.Unbind(orgName, local: true); + // Global is absent or already matches; ensure global is set and local is clear. + if (existingBinding?.GlobalUserName is null) + { + bindingManager.Bind(orgName, userName, local: false); + } + if (existingBinding?.LocalUserName is not null) + { + bindingManager.Unbind(orgName, local: true); + } } } diff --git a/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs b/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs index 517a8c7b8..6a887a97c 100644 --- a/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs +++ b/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs @@ -16,6 +16,9 @@ public class TestGitConfiguration : IGitConfiguration public IDictionary> Local { get; set; } = new Dictionary>(GitConfigurationKeyComparer.Instance); + public int SetCallCount { get; private set; } + public int UnsetCallCount { get; private set; } + #region IGitConfiguration public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCallback cb) @@ -68,6 +71,7 @@ public bool TryGet(GitConfigurationLevel level, GitConfigurationType type, strin public void Set(GitConfigurationLevel level, string name, string value) { + SetCallCount++; IDictionary> dict = GetDictionary(level); if (!dict.TryGetValue(name, out var values)) @@ -107,6 +111,7 @@ public void Add(GitConfigurationLevel level, string name, string value) public void Unset(GitConfigurationLevel level, string name) { + UnsetCallCount++; IDictionary> dict = GetDictionary(level); // Simulate git