From 30a3c7c317002b22e39e811330fe0dbba2b17053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Hofman?= Date: Mon, 6 Nov 2023 15:21:24 +0100 Subject: [PATCH] Naming convention for private properties. Review fixes --- CodingConventions.md | 24 ++++++ .../ConnectionPoolManagerSwitchTest.cs | 5 +- .../UnitTests/ConnectionPoolManagerTest.cs | 85 +++++++++---------- Snowflake.Data.Tests/Util/PoolConfig.cs | 2 +- .../Client/SnowflakeDbConnectionPool.cs | 30 +++---- Snowflake.Data/Core/Session/SessionPool.cs | 27 +++--- 6 files changed, 100 insertions(+), 73 deletions(-) diff --git a/CodingConventions.md b/CodingConventions.md index 19ca8fc75..41b55906a 100644 --- a/CodingConventions.md +++ b/CodingConventions.md @@ -85,6 +85,18 @@ public class ExampleClass } ``` +#### Public property + +Use PascalCase, eg. `SomeProperty`. + +```csharp +public ExampleProperty +{ + get => _exampleProperty; + set => _exampleProperty = value; +} +``` + ### Local variables Use camelCase, eg. `someVariable`. @@ -105,6 +117,18 @@ Use PascalCase, eg. `SomeConst`. } ``` +#### Private or internal property + +Use camelCase, eg. `someProperty`. + +```csharp +private exampleProperty +{ + get; + set; +} +``` + ### Method names Use PascalCase, eg. `SomeMethod` for all methods (normal, object members, static members, public, internal, private). diff --git a/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerSwitchTest.cs b/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerSwitchTest.cs index bf40a7710..097798361 100644 --- a/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerSwitchTest.cs +++ b/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerSwitchTest.cs @@ -12,12 +12,15 @@ public class ConnectionPoolManagerSwitchTest [Test] public void TestRevertPoolToPreviousVersion() { + // act SnowflakeDbConnectionPool.SetOldConnectionPoolVersion(); + // assert var sessionPool1 = SnowflakeDbConnectionPool.GetPool(_connectionString1); var sessionPool2 = SnowflakeDbConnectionPool.GetPool(_connectionString2); Assert.AreEqual(ConnectionPoolType.SingleConnectionCache, SnowflakeDbConnectionPool.GetConnectionPoolVersion()); Assert.AreEqual(sessionPool1, sessionPool2); } } -} \ No newline at end of file +} + \ No newline at end of file diff --git a/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerTest.cs b/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerTest.cs index 82458023e..908a919a6 100644 --- a/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerTest.cs @@ -19,8 +19,8 @@ namespace Snowflake.Data.Tests.UnitTests class ConnectionPoolManagerTest { private readonly ConnectionPoolManager _connectionPoolManager = new ConnectionPoolManager(); - private readonly string _connectionString1 = "database=D1;warehouse=W1;account=A1;user=U1;password=P1;role=R1;"; - private readonly string _connectionString2 = "database=D2;warehouse=W2;account=A2;user=U2;password=P2;role=R2;"; + private const string ConnectionString1 = "database=D1;warehouse=W1;account=A1;user=U1;password=P1;role=R1;"; + private const string ConnectionString2 = "database=D2;warehouse=W2;account=A2;user=U2;password=P2;role=R2;"; private readonly SecureString _password = new SecureString(); private static PoolConfig s_poolConfig; @@ -29,35 +29,35 @@ public static void BeforeAllTests() { s_poolConfig = new PoolConfig(); SnowflakeDbConnectionPool.SetConnectionPoolVersion(ConnectionPoolType.MultipleConnectionPool); - SessionPool.SessionFactory = new MockSessionFactory(); + SessionPool.sessionFactory = new MockSessionFactory(); } [OneTimeTearDown] public void AfterAllTests() { s_poolConfig.Reset(); - SessionPool.SessionFactory = new SessionFactory(); + SessionPool.sessionFactory = new SessionFactory(); } [Test] public void TestPoolManagerReturnsSessionPoolForGivenConnectionString() { // Act - var sessionPool = _connectionPoolManager.GetPool(_connectionString1, _password); + var sessionPool = _connectionPoolManager.GetPool(ConnectionString1, _password); // Assert - Assert.AreEqual(_connectionString1, sessionPool.ConnectionString); - Assert.AreEqual(_password, sessionPool.Password); + Assert.AreEqual(ConnectionString1, sessionPool._connectionString); + Assert.AreEqual(_password, sessionPool._password); } [Test] public void TestPoolManagerReturnsSamePoolForGivenConnectionString() { // Arrange - var anotherConnectionString = _connectionString1; + var anotherConnectionString = ConnectionString1; // Act - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); var sessionPool2 = _connectionPoolManager.GetPool(anotherConnectionString, _password); // Assert @@ -68,16 +68,16 @@ public void TestPoolManagerReturnsSamePoolForGivenConnectionString() public void TestDifferentPoolsAreReturnedForDifferentConnectionStrings() { // Arrange - Assert.AreNotSame(_connectionString1, _connectionString2); + Assert.AreNotSame(ConnectionString1, ConnectionString2); // Act - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); - var sessionPool2 = _connectionPoolManager.GetPool(_connectionString2, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); + var sessionPool2 = _connectionPoolManager.GetPool(ConnectionString2, _password); // Assert Assert.AreNotSame(sessionPool1, sessionPool2); - Assert.AreEqual(_connectionString1, sessionPool1.ConnectionString); - Assert.AreEqual(_connectionString2, sessionPool2.ConnectionString); + Assert.AreEqual(ConnectionString1, sessionPool1._connectionString); + Assert.AreEqual(ConnectionString2, sessionPool2._connectionString); } @@ -85,10 +85,10 @@ public void TestDifferentPoolsAreReturnedForDifferentConnectionStrings() public void TestGetSessionWorksForSpecifiedConnectionString() { // Act - var sfSession = _connectionPoolManager.GetSession(_connectionString1, _password); + var sfSession = _connectionPoolManager.GetSession(ConnectionString1, _password); // Assert - Assert.AreEqual(_connectionString1, sfSession.ConnectionString); + Assert.AreEqual(ConnectionString1, sfSession.ConnectionString); Assert.AreEqual(_password, sfSession.Password); } @@ -96,10 +96,10 @@ public void TestGetSessionWorksForSpecifiedConnectionString() public async Task TestGetSessionAsyncWorksForSpecifiedConnectionString() { // Act - var sfSession = await _connectionPoolManager.GetSessionAsync(_connectionString1, _password, CancellationToken.None); + var sfSession = await _connectionPoolManager.GetSessionAsync(ConnectionString1, _password, CancellationToken.None); // Assert - Assert.AreEqual(_connectionString1, sfSession.ConnectionString); + Assert.AreEqual(ConnectionString1, sfSession.ConnectionString); Assert.AreEqual(_password, sfSession.Password); } @@ -108,10 +108,10 @@ public async Task TestGetSessionAsyncWorksForSpecifiedConnectionString() public void TestCountingOfSessionProvidedByPool() { // Act - _connectionPoolManager.GetSession(_connectionString1, _password); + _connectionPoolManager.GetSession(ConnectionString1, _password); // Assert - var sessionPool = _connectionPoolManager.GetPool(_connectionString1, _password); + var sessionPool = _connectionPoolManager.GetPool(ConnectionString1, _password); Assert.AreEqual(1, sessionPool.GetCurrentPoolSize()); } @@ -119,13 +119,13 @@ public void TestCountingOfSessionProvidedByPool() public void TestCountingOfSessionReturnedBackToPool() { // Arrange - var sfSession = _connectionPoolManager.GetSession(_connectionString1, _password); + var sfSession = _connectionPoolManager.GetSession(ConnectionString1, _password); // Act _connectionPoolManager.AddSession(sfSession); // Assert - var sessionPool = _connectionPoolManager.GetPool(_connectionString1, _password); + var sessionPool = _connectionPoolManager.GetPool(ConnectionString1, _password); Assert.AreEqual(1, sessionPool.GetCurrentPoolSize()); } @@ -133,8 +133,8 @@ public void TestCountingOfSessionReturnedBackToPool() public void TestSetMaxPoolSizeForAllPools() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); - var sessionPool2 = _connectionPoolManager.GetPool(_connectionString2, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); + var sessionPool2 = _connectionPoolManager.GetPool(ConnectionString2, _password); // Act _connectionPoolManager.SetMaxPoolSize(3); @@ -148,8 +148,8 @@ public void TestSetMaxPoolSizeForAllPools() public void TestSetTimeoutForAllPools() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); - var sessionPool2 = _connectionPoolManager.GetPool(_connectionString2, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); + var sessionPool2 = _connectionPoolManager.GetPool(ConnectionString2, _password); // Act _connectionPoolManager.SetTimeout(3000); @@ -163,7 +163,7 @@ public void TestSetTimeoutForAllPools() public void TestSetPoolingDisabledForAllPools() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); // Act _connectionPoolManager.SetPooling(false); @@ -176,7 +176,7 @@ public void TestSetPoolingDisabledForAllPools() public void TestSetPoolingEnabledBack() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); _connectionPoolManager.SetPooling(false); // Act @@ -190,8 +190,8 @@ public void TestSetPoolingEnabledBack() public void TestGetPoolingOnManagerLevelWhenNotAllPoolsEqual() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); - var sessionPool2 = _connectionPoolManager.GetPool(_connectionString2, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); + var sessionPool2 = _connectionPoolManager.GetPool(ConnectionString2, _password); sessionPool1.SetPooling(true); sessionPool2.SetPooling(false); @@ -206,8 +206,8 @@ public void TestGetPoolingOnManagerLevelWhenNotAllPoolsEqual() public void TestGetPoolingOnManagerLevelWorksWhenAllPoolsEqual() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); - var sessionPool2 = _connectionPoolManager.GetPool(_connectionString2, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); + var sessionPool2 = _connectionPoolManager.GetPool(ConnectionString2, _password); sessionPool1.SetPooling(true); sessionPool2.SetPooling(true); @@ -219,8 +219,8 @@ public void TestGetPoolingOnManagerLevelWorksWhenAllPoolsEqual() public void TestGetTimeoutOnManagerLevelWhenNotAllPoolsEqual() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); - var sessionPool2 = _connectionPoolManager.GetPool(_connectionString2, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); + var sessionPool2 = _connectionPoolManager.GetPool(ConnectionString2, _password); sessionPool1.SetTimeout(299); sessionPool2.SetTimeout(1313); @@ -235,8 +235,8 @@ public void TestGetTimeoutOnManagerLevelWhenNotAllPoolsEqual() public void TestGetTimeoutOnManagerLevelWhenAllPoolsEqual() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); - var sessionPool2 = _connectionPoolManager.GetPool(_connectionString2, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); + var sessionPool2 = _connectionPoolManager.GetPool(ConnectionString2, _password); sessionPool1.SetTimeout(3600); sessionPool2.SetTimeout(3600); @@ -248,8 +248,8 @@ public void TestGetTimeoutOnManagerLevelWhenAllPoolsEqual() public void TestGetMaxPoolSizeOnManagerLevelWhenNotAllPoolsEqual() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); - var sessionPool2 = _connectionPoolManager.GetPool(_connectionString2, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); + var sessionPool2 = _connectionPoolManager.GetPool(ConnectionString2, _password); sessionPool1.SetMaxPoolSize(1); sessionPool2.SetMaxPoolSize(17); @@ -264,8 +264,8 @@ public void TestGetMaxPoolSizeOnManagerLevelWhenNotAllPoolsEqual() public void TestGetMaxPoolSizeOnManagerLevelWhenAllPoolsEqual() { // Arrange - var sessionPool1 = _connectionPoolManager.GetPool(_connectionString1, _password); - var sessionPool2 = _connectionPoolManager.GetPool(_connectionString2, _password); + var sessionPool1 = _connectionPoolManager.GetPool(ConnectionString1, _password); + var sessionPool2 = _connectionPoolManager.GetPool(ConnectionString2, _password); sessionPool1.SetMaxPoolSize(33); sessionPool2.SetMaxPoolSize(33); @@ -277,8 +277,8 @@ public void TestGetMaxPoolSizeOnManagerLevelWhenAllPoolsEqual() public void TestGetCurrentPoolSizeThrowsExceptionWhenNotAllPoolsEqual() { // Arrange - EnsurePoolSize(_connectionString1, 2); - EnsurePoolSize(_connectionString2, 3); + EnsurePoolSize(ConnectionString1, 2); + EnsurePoolSize(ConnectionString2, 3); // Act/Assert var exception = Assert.Throws(() => _connectionPoolManager.GetCurrentPoolSize()); @@ -320,5 +320,4 @@ public SFSession NewSession(string connectionString, SecureString password) return mockSfSession.Object; } } - } diff --git a/Snowflake.Data.Tests/Util/PoolConfig.cs b/Snowflake.Data.Tests/Util/PoolConfig.cs index b4fa6bc55..078b6e359 100644 --- a/Snowflake.Data.Tests/Util/PoolConfig.cs +++ b/Snowflake.Data.Tests/Util/PoolConfig.cs @@ -24,10 +24,10 @@ public PoolConfig() public void Reset() { + SnowflakeDbConnectionPool.SetConnectionPoolVersion(_connectionPoolType); SnowflakeDbConnectionPool.SetMaxPoolSize(_maxPoolSize); SnowflakeDbConnectionPool.SetTimeout(_timeout); SnowflakeDbConnectionPool.SetPooling(_pooling); - SnowflakeDbConnectionPool.SetConnectionPoolVersion(_connectionPoolType); } } } diff --git a/Snowflake.Data/Client/SnowflakeDbConnectionPool.cs b/Snowflake.Data/Client/SnowflakeDbConnectionPool.cs index 46348cf61..cfdb5bfcc 100644 --- a/Snowflake.Data/Client/SnowflakeDbConnectionPool.cs +++ b/Snowflake.Data/Client/SnowflakeDbConnectionPool.cs @@ -19,7 +19,7 @@ public class SnowflakeDbConnectionPool private static IConnectionManager s_connectionManager; private const ConnectionPoolType DefaultConnectionPoolType = ConnectionPoolType.SingleConnectionCache; // TODO: set to MultipleConnectionPool once development of entire ConnectionPoolManager epic is complete - private static IConnectionManager ConnectionManager + private static IConnectionManager connectionManager { get { @@ -33,73 +33,73 @@ private static IConnectionManager ConnectionManager internal static SFSession GetSession(string connectionString, SecureString password) { s_logger.Debug($"SnowflakeDbConnectionPool::GetSession"); - return ConnectionManager.GetSession(connectionString, password); + return connectionManager.GetSession(connectionString, password); } internal static Task GetSessionAsync(string connectionString, SecureString password, CancellationToken cancellationToken) { s_logger.Debug($"SnowflakeDbConnectionPool::GetSessionAsync"); - return ConnectionManager.GetSessionAsync(connectionString, password, cancellationToken); + return connectionManager.GetSessionAsync(connectionString, password, cancellationToken); } internal static SessionPool GetPool(string connectionString) { s_logger.Debug($"SnowflakeDbConnectionPool::GetPool"); - return ConnectionManager.GetPool(connectionString); + return connectionManager.GetPool(connectionString); } internal static bool AddSession(SFSession session) { s_logger.Debug("SnowflakeDbConnectionPool::AddSession"); - return ConnectionManager.AddSession(session); + return connectionManager.AddSession(session); } public static void ClearAllPools() { s_logger.Debug("SnowflakeDbConnectionPool::ClearAllPools"); - ConnectionManager.ClearAllPools(); + connectionManager.ClearAllPools(); } public static void SetMaxPoolSize(int maxPoolSize) { s_logger.Debug("SnowflakeDbConnectionPool::SetMaxPoolSize"); - ConnectionManager.SetMaxPoolSize(maxPoolSize); + connectionManager.SetMaxPoolSize(maxPoolSize); } public static int GetMaxPoolSize() { s_logger.Debug("SnowflakeDbConnectionPool::GetMaxPoolSize"); - return ConnectionManager.GetMaxPoolSize(); + return connectionManager.GetMaxPoolSize(); } public static void SetTimeout(long connectionTimeout) { s_logger.Debug("SnowflakeDbConnectionPool::SetTimeout"); - ConnectionManager.SetTimeout(connectionTimeout); + connectionManager.SetTimeout(connectionTimeout); } public static long GetTimeout() { s_logger.Debug("SnowflakeDbConnectionPool::GetTimeout"); - return ConnectionManager.GetTimeout(); + return connectionManager.GetTimeout(); } public static int GetCurrentPoolSize() { s_logger.Debug("SnowflakeDbConnectionPool::GetCurrentPoolSize"); - return ConnectionManager.GetCurrentPoolSize(); + return connectionManager.GetCurrentPoolSize(); } public static bool SetPooling(bool isEnable) { s_logger.Debug("SnowflakeDbConnectionPool::SetPooling"); - return ConnectionManager.SetPooling(isEnable); + return connectionManager.SetPooling(isEnable); } public static bool GetPooling() { s_logger.Debug("SnowflakeDbConnectionPool::GetPooling"); - return ConnectionManager.GetPooling(); + return connectionManager.GetPooling(); } internal static void SetOldConnectionPoolVersion() // TODO: set to public once development of entire ConnectionPoolManager epic is complete @@ -127,9 +127,9 @@ internal static void SetConnectionPoolVersion(ConnectionPoolType requestedPoolTy internal static ConnectionPoolType GetConnectionPoolVersion() { - if (ConnectionManager != null) + if (connectionManager != null) { - switch (ConnectionManager) + switch (connectionManager) { case ConnectionCacheManager _: return ConnectionPoolType.SingleConnectionCache; case ConnectionPoolManager _: return ConnectionPoolType.MultipleConnectionPool; diff --git a/Snowflake.Data/Core/Session/SessionPool.cs b/Snowflake.Data/Core/Session/SessionPool.cs index 0103d09e7..20b2b60cd 100644 --- a/Snowflake.Data/Core/Session/SessionPool.cs +++ b/Snowflake.Data/Core/Session/SessionPool.cs @@ -23,11 +23,11 @@ sealed class SessionPool : IDisposable private long _timeout; private const int MaxPoolSize = 10; private const long Timeout = 3600; - internal string ConnectionString; - internal SecureString Password; + internal string _connectionString; + internal SecureString _password; private bool _pooling = true; private bool _allowExceedMaxPoolSize = true; - internal static ISessionFactory SessionFactory = new SessionFactory(); + private static ISessionFactory s_sessionFactory = new SessionFactory(); private SessionPool() { @@ -41,8 +41,8 @@ private SessionPool() private SessionPool(string connectionString, SecureString password) : this() { - ConnectionString = connectionString; - Password = password; + _connectionString = connectionString; + _password = password; _allowExceedMaxPoolSize = false; // TODO: SNOW-937190 } @@ -63,6 +63,11 @@ public void Dispose() ClearAllPools(); } + internal static ISessionFactory sessionFactory + { + set => s_sessionFactory = value; + } + private void CleanExpiredSessions() { s_logger.Debug("SessionPool::CleanExpiredSessions"); @@ -83,8 +88,6 @@ private void CleanExpiredSessions() internal SFSession GetSession(string connStr, SecureString password) { - s_logger.Info($"GetSession st: {new System.Diagnostics.StackTrace()}\n{connStr}"); - s_logger.Debug("SessionPool::GetSession"); if (!_pooling) return NewSession(connStr, password); @@ -101,10 +104,10 @@ internal Task GetSessionAsync(string connStr, SecureString password, return session != null ? Task.FromResult(session) : NewSessionAsync(connStr, password, cancellationToken); } - internal SFSession GetSession() => GetSession(ConnectionString, Password); + internal SFSession GetSession() => GetSession(_connectionString, _password); internal Task GetSessionAsync(CancellationToken cancellationToken) => - GetSessionAsync(ConnectionString, Password, cancellationToken); + GetSessionAsync(_connectionString, _password, cancellationToken); private SFSession GetIdleSession(string connStr) { @@ -139,7 +142,7 @@ private SFSession NewSession(String connectionString, SecureString password) s_logger.Debug("SessionPool::NewSession"); try { - var session = SessionFactory.NewSession(connectionString, password); + var session = s_sessionFactory.NewSession(connectionString, password); session.Open(); return session; } @@ -159,7 +162,7 @@ private SFSession NewSession(String connectionString, SecureString password) private Task NewSessionAsync(String connectionString, SecureString password, CancellationToken cancellationToken) { s_logger.Debug("SessionPool::NewSessionAsync"); - var session = SessionFactory.NewSession(connectionString, password); + var session = s_sessionFactory.NewSession(connectionString, password); return session .OpenAsync(cancellationToken) .ContinueWith(previousTask => @@ -179,8 +182,6 @@ private Task NewSessionAsync(String connectionString, SecureString pa internal bool AddSession(SFSession session) { - s_logger.Info($"AddSession st: {new System.Diagnostics.StackTrace()}\n{session.ConnectionString}"); - s_logger.Debug("SessionPool::AddSession"); if (!_pooling) return false;