Remove EndUserAction on pg error from ReadMessageLong (#4924)

vonzshik · web-flow · commit 99e7324ffce9 · 2023-03-15T01:40:10.000+03:00
diff --git a/src/Npgsql/BackendMessages/AuthenticationMessages.cs b/src/Npgsql/BackendMessages/AuthenticationMessages.cs
@@ -40,7 +40,7 @@ sealed class AuthenticationMD5PasswordMessage  : AuthenticationRequestMessage
 {
     internal override AuthenticationRequestType AuthRequestType => AuthenticationRequestType.AuthenticationMD5Password;
 
-    internal byte[] Salt { get; private set; }
+    internal byte[] Salt { get; }
 
     internal static AuthenticationMD5PasswordMessage Load(NpgsqlReadBuffer buf)
     {
@@ -75,7 +75,7 @@ sealed class AuthenticationGSSContinueMessage : AuthenticationRequestMessage
 {
     internal override AuthenticationRequestType AuthRequestType => AuthenticationRequestType.AuthenticationGSSContinue;
 
-    internal byte[] AuthenticationData { get; private set; }
+    internal byte[] AuthenticationData { get; }
 
     internal static AuthenticationGSSContinueMessage Load(NpgsqlReadBuffer buf, int len)
     {
diff --git a/src/Npgsql/BackendMessages/BackendKeyDataMessage.cs b/src/Npgsql/BackendMessages/BackendKeyDataMessage.cs
@@ -6,8 +6,8 @@ sealed class BackendKeyDataMessage : IBackendMessage
 {
     public BackendMessageCode Code => BackendMessageCode.BackendKeyData;
 
-    internal int BackendProcessId { get; private set; }
-    internal int BackendSecretKey { get; private set; }
+    internal int BackendProcessId { get; }
+    internal int BackendSecretKey { get; }
 
     internal BackendKeyDataMessage(NpgsqlReadBuffer buf)
     {
diff --git a/src/Npgsql/Internal/NpgsqlConnector.cs b/src/Npgsql/Internal/NpgsqlConnector.cs
@@ -1307,8 +1307,6 @@ internal ValueTask<IBackendMessage> ReadMessage(
         case BackendMessageCode.ErrorResponse:
             ReadBuffer.ReadPosition--;
             return ReadMessageLong(async, dataRowLoadingMode, readingNotifications: false)!;
-        case BackendMessageCode.ReadyForQuery:
-            break;
         }
 
         PGUtil.ValidateBackendMessageCode(messageCode);
@@ -1444,10 +1442,6 @@ internal ValueTask<IBackendMessage> ReadMessage(
         }
         catch (PostgresException e)
         {
-            // TODO: move it up the stack, like #3126 did (relevant for non-command-execution scenarios, like COPY)
-            if (CurrentReader is null)
-                EndUserAction();
-
             if (e.SqlState == PostgresErrorCodes.QueryCanceled && PostgresCancellationPerformed)
             {
                 // The query could be canceled because of a user cancellation or a timeout - raise the proper exception.
diff --git a/src/Npgsql/Internal/TypeHandling/TypeMappingInfo.cs b/src/Npgsql/Internal/TypeHandling/TypeMappingInfo.cs
@@ -5,7 +5,7 @@
 
 namespace Npgsql.Internal.TypeHandling;
 
-public class TypeMappingInfo
+public sealed class TypeMappingInfo
 {
     public TypeMappingInfo(NpgsqlDbType? npgsqlDbType, string? dataTypeName, Type clrType)
         => (NpgsqlDbType, DataTypeName, ClrTypes) = (npgsqlDbType, dataTypeName, new[] { clrType });
diff --git a/src/Npgsql/NameTranslation/NpgsqlNullNameTranslator.cs b/src/Npgsql/NameTranslation/NpgsqlNullNameTranslator.cs
@@ -5,7 +5,7 @@ namespace Npgsql.NameTranslation;
 /// <summary>
 /// A name translator which preserves CLR names (e.g. SomeClass) when mapping names to the database.
 /// </summary>
-public class NpgsqlNullNameTranslator : INpgsqlNameTranslator
+public sealed class NpgsqlNullNameTranslator : INpgsqlNameTranslator
 {
     /// <summary>
     /// Given a CLR type name (e.g class, struct, enum), translates its name to a database type name.
diff --git a/src/Npgsql/NameTranslation/NpgsqlSnakeCaseNameTranslator.cs b/src/Npgsql/NameTranslation/NpgsqlSnakeCaseNameTranslator.cs
@@ -9,7 +9,7 @@ namespace Npgsql.NameTranslation;
 /// A name translator which converts standard CLR names (e.g. SomeClass) to snake-case database
 /// names (some_class)
 /// </summary>
-public class NpgsqlSnakeCaseNameTranslator : INpgsqlNameTranslator
+public sealed class NpgsqlSnakeCaseNameTranslator : INpgsqlNameTranslator
 {
     /// <summary>
     /// Creates a new <see cref="NpgsqlSnakeCaseNameTranslator"/>.
diff --git a/src/Npgsql/NpgsqlBinaryImporter.cs b/src/Npgsql/NpgsqlBinaryImporter.cs
@@ -551,7 +551,6 @@ async ValueTask CloseAsync(bool async, CancellationToken cancellationToken = def
             throw new Exception("Invalid state: " + _state);
         }
 
-        _connector.EndUserAction();
         Cleanup();
     }
 
@@ -566,8 +565,9 @@ void Cleanup()
 
         if (connector != null)
         {
+            connector.EndUserAction();
             connector.CurrentCopyOperation = null;
-            _connector.Connection?.EndBindingScope(ConnectorBindingScope.Copy);
+            connector.Connection?.EndBindingScope(ConnectorBindingScope.Copy);
             _connector = null;
         }
 
diff --git a/src/Npgsql/NpgsqlDataSource.cs b/src/Npgsql/NpgsqlDataSource.cs
@@ -41,7 +41,7 @@ public abstract class NpgsqlDataSource : DbDataSource
     /// <summary>
     /// Information about PostgreSQL and PostgreSQL-like databases (e.g. type definitions, capabilities...).
     /// </summary>
-    internal NpgsqlDatabaseInfo DatabaseInfo { get; set; } = null!; // Initialized at bootstrapping
+    internal NpgsqlDatabaseInfo DatabaseInfo { get; private set; } = null!; // Initialized at bootstrapping
 
     internal Func<NpgsqlConnector, SslMode, NpgsqlTimeout, bool, bool, Task>? EncryptionNegotiator { get; }
     internal RemoteCertificateValidationCallback? UserCertificateValidationCallback { get; }
diff --git a/src/Npgsql/NpgsqlRawCopyStream.cs b/src/Npgsql/NpgsqlRawCopyStream.cs
@@ -372,7 +372,6 @@ async Task Cancel(bool async)
             }
             catch (PostgresException e)
             {
-                _connector.EndUserAction();
                 Cleanup();
 
                 if (e.SqlState != PostgresErrorCodes.QueryCanceled)
@@ -442,7 +441,6 @@ async ValueTask DisposeAsync(bool disposing, bool async)
         }
         finally
         {
-            _connector.EndUserAction();
             Cleanup();
         }
     }
@@ -452,6 +450,7 @@ void Cleanup()
     {
         Debug.Assert(!_isDisposed);
         LogMessages.CopyOperationCompleted(_copyLogger, _connector.Id);
+        _connector.EndUserAction();
         _connector.CurrentCopyOperation = null;
         _connector.Connection?.EndBindingScope(ConnectorBindingScope.Copy);
         _connector = null;
diff --git a/src/Npgsql/NpgsqlTypes/PgNameAttribute.cs b/src/Npgsql/NpgsqlTypes/PgNameAttribute.cs
@@ -18,14 +18,12 @@ public class PgNameAttribute : Attribute
     /// <summary>
     /// The name of PostgreSQL field that corresponds to this CLR property or field
     /// </summary>
-    public string PgName { get; private set; }
+    public string PgName { get; }
 
     /// <summary>
     /// Indicates that this property or field corresponds to a PostgreSQL field with the specified name
     /// </summary>
     /// <param name="pgName">The name of PostgreSQL field that corresponds to this CLR property or field</param>
     public PgNameAttribute(string pgName)
-    {
-        PgName = pgName;
-    }
+        => PgName = pgName;
 }
diff --git a/src/Npgsql/Util/PGUtil.cs b/src/Npgsql/Util/PGUtil.cs
@@ -25,7 +25,7 @@ static Statics()
         LegacyTimestampBehavior = AppContext.TryGetSwitch("Npgsql.EnableLegacyTimestampBehavior", out var enabled) && enabled;
         DisableDateTimeInfinityConversions = AppContext.TryGetSwitch("Npgsql.DisableDateTimeInfinityConversions", out enabled) && enabled;
     }
-    
+
     internal static T Expect<T>(IBackendMessage msg, NpgsqlConnector connector)
     {
         if (msg.GetType() != typeof(T))

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ sealed class AuthenticationMD5PasswordMessage : AuthenticationRequestMessage`
`40`	`40`	`{`
`41`	`41`	`internal override AuthenticationRequestType AuthRequestType => AuthenticationRequestType.AuthenticationMD5Password;`
`42`	`42`
`43`		`- internal byte[] Salt { get; private set; }`
	`43`	`+ internal byte[] Salt { get; }`
`44`	`44`
`45`	`45`	`internal static AuthenticationMD5PasswordMessage Load(NpgsqlReadBuffer buf)`
`46`	`46`	`{`
`@@ -75,7 +75,7 @@ sealed class AuthenticationGSSContinueMessage : AuthenticationRequestMessage`
`75`	`75`	`{`
`76`	`76`	`internal override AuthenticationRequestType AuthRequestType => AuthenticationRequestType.AuthenticationGSSContinue;`
`77`	`77`
`78`		`- internal byte[] AuthenticationData { get; private set; }`
	`78`	`+ internal byte[] AuthenticationData { get; }`
`79`	`79`
`80`	`80`	`internal static AuthenticationGSSContinueMessage Load(NpgsqlReadBuffer buf, int len)`
`81`	`81`	`{`
Original file line number	Diff line number	Diff line change
`@@ -6,8 +6,8 @@ sealed class BackendKeyDataMessage : IBackendMessage`
`6`	`6`	`{`
`7`	`7`	`public BackendMessageCode Code => BackendMessageCode.BackendKeyData;`
`8`	`8`
`9`		`- internal int BackendProcessId { get; private set; }`
`10`		`- internal int BackendSecretKey { get; private set; }`
	`9`	`+ internal int BackendProcessId { get; }`
	`10`	`+ internal int BackendSecretKey { get; }`
`11`	`11`
`12`	`12`	`internal BackendKeyDataMessage(NpgsqlReadBuffer buf)`
`13`	`13`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1307,8 +1307,6 @@ internal ValueTask<IBackendMessage> ReadMessage(`
`1307`	`1307`	`case BackendMessageCode.ErrorResponse:`
`1308`	`1308`	`ReadBuffer.ReadPosition--;`
`1309`	`1309`	`return ReadMessageLong(async, dataRowLoadingMode, readingNotifications: false)!;`
`1310`		`- case BackendMessageCode.ReadyForQuery:`
`1311`		`- break;`
`1312`	`1310`	`}`
`1313`	`1311`
`1314`	`1312`	`PGUtil.ValidateBackendMessageCode(messageCode);`
`@@ -1444,10 +1442,6 @@ internal ValueTask<IBackendMessage> ReadMessage(`
`1444`	`1442`	`}`
`1445`	`1443`	`catch (PostgresException e)`
`1446`	`1444`	`{`
`1447`		`- // TODO: move it up the stack, like #3126 did (relevant for non-command-execution scenarios, like COPY)`
`1448`		`- if (CurrentReader is null)`
`1449`		`- EndUserAction();`
`1450`		`-`
`1451`	`1445`	`if (e.SqlState == PostgresErrorCodes.QueryCanceled && PostgresCancellationPerformed)`
`1452`	`1446`	`{`
`1453`	`1447`	`// The query could be canceled because of a user cancellation or a timeout - raise the proper exception.`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@`
`5`	`5`
`6`	`6`	`namespace Npgsql.Internal.TypeHandling;`
`7`	`7`
`8`		`-public class TypeMappingInfo`
	`8`	`+public sealed class TypeMappingInfo`
`9`	`9`	`{`
`10`	`10`	`public TypeMappingInfo(NpgsqlDbType? npgsqlDbType, string? dataTypeName, Type clrType)`
`11`	`11`	`=> (NpgsqlDbType, DataTypeName, ClrTypes) = (npgsqlDbType, dataTypeName, new[] { clrType });`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@ namespace Npgsql.NameTranslation;`
`5`	`5`	`/// <summary>`
`6`	`6`	`/// A name translator which preserves CLR names (e.g. SomeClass) when mapping names to the database.`
`7`	`7`	`/// </summary>`
`8`		`-public class NpgsqlNullNameTranslator : INpgsqlNameTranslator`
	`8`	`+public sealed class NpgsqlNullNameTranslator : INpgsqlNameTranslator`
`9`	`9`	`{`
`10`	`10`	`/// <summary>`
`11`	`11`	`/// Given a CLR type name (e.g class, struct, enum), translates its name to a database type name.`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ namespace Npgsql.NameTranslation;`
`9`	`9`	`/// A name translator which converts standard CLR names (e.g. SomeClass) to snake-case database`
`10`	`10`	`/// names (some_class)`
`11`	`11`	`/// </summary>`
`12`		`-public class NpgsqlSnakeCaseNameTranslator : INpgsqlNameTranslator`
	`12`	`+public sealed class NpgsqlSnakeCaseNameTranslator : INpgsqlNameTranslator`
`13`	`13`	`{`
`14`	`14`	`/// <summary>`
`15`	`15`	`/// Creates a new <see cref="NpgsqlSnakeCaseNameTranslator"/>.`
Original file line number	Diff line number	Diff line change
`@@ -551,7 +551,6 @@ async ValueTask CloseAsync(bool async, CancellationToken cancellationToken = def`
`551`	`551`	`throw new Exception("Invalid state: " + _state);`
`552`	`552`	`}`
`553`	`553`
`554`		`- _connector.EndUserAction();`
`555`	`554`	`Cleanup();`
`556`	`555`	`}`
`557`	`556`
`@@ -566,8 +565,9 @@ void Cleanup()`
`566`	`565`
`567`	`566`	`if (connector != null)`
`568`	`567`	`{`
	`568`	`+ connector.EndUserAction();`
`569`	`569`	`connector.CurrentCopyOperation = null;`
`570`		`- _connector.Connection?.EndBindingScope(ConnectorBindingScope.Copy);`
	`570`	`+ connector.Connection?.EndBindingScope(ConnectorBindingScope.Copy);`
`571`	`571`	`_connector = null;`
`572`	`572`	`}`
`573`	`573`
Original file line number	Diff line number	Diff line change
`@@ -372,7 +372,6 @@ async Task Cancel(bool async)`
`372`	`372`	`}`
`373`	`373`	`catch (PostgresException e)`
`374`	`374`	`{`
`375`		`- _connector.EndUserAction();`
`376`	`375`	`Cleanup();`
`377`	`376`
`378`	`377`	`if (e.SqlState != PostgresErrorCodes.QueryCanceled)`
`@@ -442,7 +441,6 @@ async ValueTask DisposeAsync(bool disposing, bool async)`
`442`	`441`	`}`
`443`	`442`	`finally`
`444`	`443`	`{`
`445`		`- _connector.EndUserAction();`
`446`	`444`	`Cleanup();`
`447`	`445`	`}`
`448`	`446`	`}`
`@@ -452,6 +450,7 @@ void Cleanup()`
`452`	`450`	`{`
`453`	`451`	`Debug.Assert(!_isDisposed);`
`454`	`452`	`LogMessages.CopyOperationCompleted(_copyLogger, _connector.Id);`
	`453`	`+ _connector.EndUserAction();`
`455`	`454`	`_connector.CurrentCopyOperation = null;`
`456`	`455`	`_connector.Connection?.EndBindingScope(ConnectorBindingScope.Copy);`
`457`	`456`	`_connector = null;`