Address review comments

Azure · Feb 2, 2023 · 70b8375 · 70b8375
1 parent 58eb20d
commit 70b8375
Show file tree

Hide file tree

Showing 11 changed files with 39 additions and 55 deletions.
diff --git a/src/Accounts/Accounts.Test/AutosaveTests.cs b/src/Accounts/Accounts.Test/AutosaveTests.cs
@@ -41,7 +41,6 @@ public AutosaveTests(ITestOutputHelper output)
             XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output));
             commandRuntimeMock = new MockCommandRuntime();
             dataStore = new MemoryDataStore();
-            ResetState();
             keyStore = SetMockedAzKeyStore();
         }
 

diff --git a/src/Accounts/Accounts.Test/ContextCmdletTests.cs b/src/Accounts/Accounts.Test/ContextCmdletTests.cs
@@ -64,7 +64,7 @@ public ContextCmdletTests(ITestOutputHelper output)
             Mock<IStorage> storageMocker = new Mock<IStorage>();
             AzKeyStore azKeyStore = null;
             string profilePath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), Resources.AzureDirectoryName);
-        azKeyStore = new AzKeyStore(profilePath, AzureSession.Instance.KeyStoreFile, storageMocker.Object);
+            azKeyStore = new AzKeyStore(profilePath, AzureSession.Instance.KeyStoreFile, true, storageMocker.Object);
             AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => azKeyStore, true);
         }
 

diff --git a/src/Accounts/Accounts.Test/ProfileCmdletTests.cs b/src/Accounts/Accounts.Test/ProfileCmdletTests.cs
@@ -55,7 +55,7 @@ private AzKeyStore SetMockedAzKeyStore()
             storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object);
             storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]);
             storageMocker.Setup(f => f.WriteData(It.IsAny<byte[]>())).Callback((byte[] s) => { });
-            var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", storageMocker.Object);
+            var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", false, storageMocker.Object);
             return keyStore;
         }
 

diff --git a/src/Accounts/Authentication.Test/StorageHelperTest.cs b/src/Accounts/Authentication.Test/StorageHelperTest.cs
@@ -72,11 +72,7 @@ public void SaveToStorageTest()
 
             var helper = StorageHelper.GetStorageHelperAsync(true, keyStoreFileName, profilePath
                 , keystoreMocker.Object, storageMocker.Object).GetAwaiter().GetResult();
-            var args = new KeyStoreNotificationArgs()
-            {
-                KeyStore = keystoreMocker.Object
-            };
-            helper.WriteToCachedStorage(args);
+            helper.WriteToCachedStorage(keystoreMocker.Object);
 
             string actual = Encoding.UTF8.GetString(storageChecker.ToArray());
             Assert.Equal(EXPECTED, actual);

diff --git a/src/Accounts/Authentication/KeyStore/AzKeyStore.cs b/src/Accounts/Authentication/KeyStore/AzKeyStore.cs
@@ -23,11 +23,11 @@ public class AzKeyStore : IDisposable
         public string FileName { get; set; }
         public string Directory { get; set; }
 
-        private IKeyCache _inMemoryStore = null;
-        public IKeyCache InMemoryStore
+        private IKeyCache _inMemoryKeyCache = null;
+        public IKeyCache InMemoryKeyCache
         {
-            get => _inMemoryStore;
-            set => _inMemoryStore = value;
+            get => _inMemoryKeyCache;
+            set => _inMemoryKeyCache = value;
         }
 
         private IStorage inputStorage = null;
@@ -38,50 +38,43 @@ public bool IsProtected
             private set;
         }
 
-        public bool SyncToStorage
-        {
-            get; set;
-        }
-
         public AzKeyStore(string directory, string fileName, bool enableContextAutoSaving = true, IStorage storage = null)
         {
-            InMemoryStore = new InMemoryKeyCache();
-
-            SyncToStorage = enableContextAutoSaving;
+            InMemoryKeyCache = new InMemoryKeyCache();
 
-            if (SyncToStorage)
+            if (enableContextAutoSaving)
             {
-                InMemoryStore.SetBeforeAccess(LoadStorage);
-                InMemoryStore.SetOnUpdate(UpdateStorage);
+                InMemoryKeyCache.SetBeforeAccess(LoadStorage);
+                InMemoryKeyCache.SetOnUpdate(UpdateStorage);
             }
 
             FileName = fileName;
             Directory = directory;
 
             inputStorage = storage;
 
-            InMemoryKeyCache.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name);
-            InMemoryKeyCache.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter());
+            Common.InMemoryKeyCache.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name);
+            Common.InMemoryKeyCache.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter());
         }
 
 
         private void LoadStorage(KeyStoreNotificationArgs args)
         {
-            var asyncHelper = StorageHelper.GetStorageHelperAsync(true, FileName, Directory, args.KeyStore, inputStorage);
+            var asyncHelper = StorageHelper.GetStorageHelperAsync(true, FileName, Directory, args.KeyCache, inputStorage);
             var helper = asyncHelper.GetAwaiter().GetResult();
             IsProtected = helper.IsProtected;
         }
 
         private void UpdateStorage(KeyStoreNotificationArgs args)
         {
-            var asyncHelper = StorageHelper.GetStorageHelperAsync(false, FileName, Directory, args.KeyStore, inputStorage);
+            var asyncHelper = StorageHelper.GetStorageHelperAsync(false, FileName, Directory, args.KeyCache, inputStorage);
             var helper = asyncHelper.GetAwaiter().GetResult();
-            helper.WriteToCachedStorage(args.KeyStore);
+            helper.WriteToCachedStorage(args.KeyCache);
         }
 
         public void Clear()
         {
-            InMemoryStore.Clear();
+            InMemoryKeyCache.Clear();
         }
 
         public void Dispose()
@@ -91,29 +84,29 @@ public void Dispose()
 
         public void SaveSecureString(IKeyStoreKey key, SecureString value)
         {
-            InMemoryStore.SaveKey(key, value);
+            InMemoryKeyCache.SaveKey(key, value);
         }
 
         public SecureString GetSecureString(IKeyStoreKey key)
         {
-            return InMemoryStore.GetKey<SecureString>(key);
+            return InMemoryKeyCache.GetKey<SecureString>(key);
         }
 
         public bool RemoveSecureString(IKeyStoreKey key)
         {
-            return InMemoryStore.DeleteKey(key);
+            return InMemoryKeyCache.DeleteKey(key);
         }
 
         public void EnableSyncToStorage()
         {
-            InMemoryStore.SetBeforeAccess(LoadStorage);
-            InMemoryStore.SetOnUpdate(UpdateStorage);
+            InMemoryKeyCache.SetBeforeAccess(LoadStorage);
+            InMemoryKeyCache.SetOnUpdate(UpdateStorage);
         }
 
         public void DisableSyncToStorage()
         {
-            InMemoryStore.SetBeforeAccess(null);
-            InMemoryStore.SetOnUpdate(null);
+            InMemoryKeyCache.SetBeforeAccess(null);
+            InMemoryKeyCache.SetOnUpdate(null);
         }
     }
 }
diff --git a/src/Accounts/Authentication/KeyStore/IStorage.cs b/src/Accounts/Authentication/KeyStore/IStorage.cs
@@ -11,7 +11,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 // ----------------------------------------------------------------------------------
-using System;
 
 namespace Microsoft.Azure.Commands.ResourceManager.Common
 {

diff --git a/src/Accounts/Authentication/KeyStore/IStorageHelper.cs b/src/Accounts/Authentication/KeyStore/IStorageHelper.cs
@@ -11,17 +11,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 // ----------------------------------------------------------------------------------
-using System;
 
 namespace Microsoft.Azure.Commands.ResourceManager.Common
 {
     public interface IStorageHelper
     {
         void Clear();
 
-        void LoadFromCachedStorage(IKeyCache keystore);
+        void LoadFromCachedStorage(IKeyCache keycache);
 
-        void WriteToCachedStorage(IKeyCache keystore);
+        void WriteToCachedStorage(IKeyCache keycache);
 
         bool IsProtected
         {

diff --git a/src/Accounts/Authentication/KeyStore/InMemoryKeyCache.cs b/src/Accounts/Authentication/KeyStore/InMemoryKeyCache.cs
@@ -11,14 +11,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 // ----------------------------------------------------------------------------------
-using Microsoft.Identity.Client;
 using Newtonsoft.Json;
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
-using System.Threading.Tasks;
 
 namespace Microsoft.Azure.Commands.ResourceManager.Common
 {
@@ -48,7 +46,7 @@ public void SaveKey<T>(IKeyStoreKey key, T value)
         {
             var args = new KeyStoreNotificationArgs()
             {
-                KeyStore = this
+                KeyCache = this
             };
             BeforeAccess?.Invoke(args) ;
             if (!_typeNameMap.ContainsKey(key.GetType()) || !_typeNameMap.ContainsKey(value.GetType()))
@@ -63,7 +61,7 @@ public T GetKey<T>(IKeyStoreKey key)
         {
             var args = new KeyStoreNotificationArgs()
             {
-                KeyStore = this
+                KeyCache = this
             };
             BeforeAccess?.Invoke(args);
 
@@ -88,7 +86,7 @@ public bool DeleteKey(IKeyStoreKey key)
         {
             var args = new KeyStoreNotificationArgs()
             {
-                KeyStore = this
+                KeyCache = this
             };
             BeforeAccess?.Invoke(args);
             bool ret = false;
@@ -159,7 +157,7 @@ public void Clear()
         {
             var args = new KeyStoreNotificationArgs()
             {
-                KeyStore = this
+                KeyCache = this
             };
             BeforeAccess?.Invoke(args);
             _credentials.Clear();

diff --git a/src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs b/src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs
@@ -16,7 +16,7 @@ namespace Microsoft.Azure.Commands.ResourceManager.Common
 {
     public class KeyStoreNotificationArgs
     {
-        public IKeyCache KeyStore;
+        public IKeyCache KeyCache;
     }
 
     public delegate void KeyStoreCallbak(KeyStoreNotificationArgs args);

diff --git a/src/Accounts/Authentication/KeyStore/StorageHelper.cs b/src/Accounts/Authentication/KeyStore/StorageHelper.cs
@@ -20,7 +20,7 @@
 
 namespace Microsoft.Azure.Commands.ResourceManager.Common
 {
-    public class StorageHelper : IStorageHelper
+    internal class StorageHelper : IStorageHelper
     {
         private const string KeyChainServiceName = "Microsoft.Azure.PowerShell";
 
@@ -88,7 +88,7 @@ private static StorageHelper Create(StorageCreationProperties storageCreationPro
         }
 
         #region Public API
-        public static async Task<IStorageHelper> GetStorageHelperAsync(bool async, string fileName, string directory, IKeyCache keystore, IStorage storage = null)
+        public static async Task<IStorageHelper> GetStorageHelperAsync(bool async, string fileName, string directory, IKeyCache keycache, IStorage storage = null)
         {
             StorageHelper storageHelper = null;
 
@@ -109,7 +109,7 @@ public static async Task<IStorageHelper> GetStorageHelperAsync(bool async, strin
                     storageHelper = GetFallbackStorageHelper(fileName, directory, storage);
                     storageHelper.VerifyPersistence();
                 }
-                storageHelper.LoadFromCachedStorage(keystore);
+                storageHelper.LoadFromCachedStorage(keycache);
                 asyncLock.SetValue(storageHelper);
             }
             return storageHelper;
@@ -128,7 +128,7 @@ public void Clear()
             }
         }
 
-        public void LoadFromCachedStorage(IKeyCache keystore)
+        public void LoadFromCachedStorage(IKeyCache keycache)
         {
             LogMessage(EventLogLevel.Verbose, $"Before access\nAcquiring lock for keystore");
 
@@ -152,7 +152,7 @@ public void LoadFromCachedStorage(IKeyCache keystore)
                 try
                 {
                     LogMessage(EventLogLevel.Verbose, $"Deserializing the store");
-                    keystore.Deserialize(cachedStoreData, false);
+                    keycache.Deserialize(cachedStoreData, false);
                 }
                 catch (Exception e)
                 {
@@ -165,7 +165,7 @@ public void LoadFromCachedStorage(IKeyCache keystore)
             }
         }
 
-        public void WriteToCachedStorage(IKeyCache keystore)
+        public void WriteToCachedStorage(IKeyCache keycache)
         {
             using (CreateCrossPlatLock(_storageCreationProperties))
             {
@@ -174,7 +174,7 @@ public void WriteToCachedStorage(IKeyCache keystore)
                 LogMessage(EventLogLevel.Verbose, $"After access, cache in memory HasChanged");
                 try
                 {
-                    data = keystore.Serialize();
+                    data = keycache.Serialize();
                 }
                 catch (Exception e)
                 {

diff --git a/src/Accounts/Authentication/KeyStore/StorageWrapper.cs b/src/Accounts/Authentication/KeyStore/StorageWrapper.cs
@@ -16,7 +16,7 @@
 
 namespace Microsoft.Azure.Commands.ResourceManager.Common
 {
-    class StorageWrapper : IStorage
+    internal class StorageWrapper : IStorage
     {
         public StorageCreationProperties StorageCreationProperties { get; set; }