From af4daf770750aa4bf6ca3c073d6667162c4cf4bf Mon Sep 17 00:00:00 2001 From: Krzysztof Nozderko Date: Mon, 26 Aug 2024 08:05:30 +0000 Subject: [PATCH] fix review comments --- .../IntegrationTests/SFConnectionIT.cs | 19 +++++++++---------- .../UnitTests/SFSessionTest.cs | 2 +- .../Client/SnowflakeDbConnection.cs | 2 +- Snowflake.Data/Core/Session/SFSession.cs | 15 ++++++--------- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs index c61167c25..9eaebe377 100644 --- a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs @@ -2342,22 +2342,21 @@ public void TestHangingCloseIsNotBlocking() var restRequester = new MockCloseHangingRestRequester(); var session = new SFSession("account=test;user=test;password=test", null, restRequester); session.Open(); - var watch = new Stopwatch(); + var watchClose = new Stopwatch(); + var watchClosedFinished = new Stopwatch(); // act - watch.Start(); - session.closeNonBlocking(); - watch.Stop(); - var closeDuration = watch.Elapsed.Duration(); - watch.Restart(); + watchClose.Start(); + watchClosedFinished.Start(); + session.CloseNonBlocking(); + watchClose.Stop(); Awaiter.WaitUntilConditionOrTimeout(() => restRequester.CloseRequests.Count > 0, TimeSpan.FromSeconds(15)); - watch.Stop(); - var backgroundTaskDuration = watch.Elapsed.Duration(); + watchClosedFinished.Stop(); // assert Assert.AreEqual(1, restRequester.CloseRequests.Count); - Assert.Less(closeDuration, TimeSpan.FromSeconds(5)); // close executed immediately - Assert.GreaterOrEqual(backgroundTaskDuration, TimeSpan.FromSeconds(10)); // while background task took more time + Assert.Less(watchClose.Elapsed.Duration(), TimeSpan.FromSeconds(5)); // close executed immediately + Assert.GreaterOrEqual(watchClosedFinished.Elapsed.Duration(), TimeSpan.FromSeconds(10)); // while background task took more time } } } diff --git a/Snowflake.Data.Tests/UnitTests/SFSessionTest.cs b/Snowflake.Data.Tests/UnitTests/SFSessionTest.cs index 7fa03fdb9..262122b2d 100644 --- a/Snowflake.Data.Tests/UnitTests/SFSessionTest.cs +++ b/Snowflake.Data.Tests/UnitTests/SFSessionTest.cs @@ -28,7 +28,7 @@ public void TestSessionGoneWhenCloseNonBlocking() var restRequester = new MockCloseSessionGone(); SFSession sfSession = new SFSession("account=test;user=test;password=test", null, restRequester); sfSession.Open(); - Assert.DoesNotThrow(() => sfSession.closeNonBlocking()); + Assert.DoesNotThrow(() => sfSession.CloseNonBlocking()); } [Test] diff --git a/Snowflake.Data/Client/SnowflakeDbConnection.cs b/Snowflake.Data/Client/SnowflakeDbConnection.cs index 18f3ca1c7..9acb24f06 100755 --- a/Snowflake.Data/Client/SnowflakeDbConnection.cs +++ b/Snowflake.Data/Client/SnowflakeDbConnection.cs @@ -397,7 +397,7 @@ protected override void Dispose(bool disposing) } else { - SfSession?.closeNonBlocking(); + SfSession?.CloseNonBlocking(); SfSession = null; _connectionState = ConnectionState.Closed; } diff --git a/Snowflake.Data/Core/Session/SFSession.cs b/Snowflake.Data/Core/Session/SFSession.cs index 6d1cca64b..b6a0ebf79 100755 --- a/Snowflake.Data/Core/Session/SFSession.cs +++ b/Snowflake.Data/Core/Session/SFSession.cs @@ -281,11 +281,10 @@ internal void close() stopHeartBeatForThisSession(); var closeSessionRequest = PrepareCloseSessionRequest(); PostCloseSession(closeSessionRequest, restRequester); - // Just in case the session won't be closed twice sessionToken = null; } - internal void closeNonBlocking() + internal void CloseNonBlocking() { // Nothing to do if the session is not open if (!IsEstablished()) return; @@ -293,7 +292,6 @@ internal void closeNonBlocking() stopHeartBeatForThisSession(); var closeSessionRequest = PrepareCloseSessionRequest(); Task.Run(() => PostCloseSession(closeSessionRequest, restRequester)); - // Just in case the session won't be closed twice sessionToken = null; } @@ -306,15 +304,14 @@ internal async Task CloseAsync(CancellationToken cancellationToken) var closeSessionRequest = PrepareCloseSessionRequest(); - logger.Debug($"Send async closeSessionRequest"); + logger.Debug($"Closing session async"); var response = await restRequester.PostAsync(closeSessionRequest, cancellationToken).ConfigureAwait(false); if (!response.success) { - logger.Error($"Failed to delete session {sessionId}, error ignored. Code: {response.code} Message: {response.message}"); + logger.Error($"Failed to close session {sessionId}, error ignored. Code: {response.code} Message: {response.message}"); } logger.Debug($"Session closed: {sessionId}"); - // Just in case the session won't be closed twice sessionToken = null; } @@ -322,18 +319,18 @@ private static void PostCloseSession(SFRestRequest closeSessionRequest, IRestReq { try { - logger.Debug($"Send closeSessionRequest"); + logger.Debug($"Closing session"); var response = restRequester.Post(closeSessionRequest); if (!response.success) { - logger.Error($"Failed to delete session: {closeSessionRequest.sid}, error ignored. Code: {response.code} Message: {response.message}"); + logger.Error($"Failed to close session: {closeSessionRequest.sid}, error ignored. Code: {response.code} Message: {response.message}"); } logger.Debug($"Session closed: {closeSessionRequest.sid}"); } catch (Exception) { - logger.Error($"Failed to delete session: {closeSessionRequest.sid}, because of exception."); + logger.Error($"Failed to close session: {closeSessionRequest.sid}, because of exception."); throw; } }