Skip to content

Commit

Permalink
Call sync connection close instead of async (#26942)
Browse files Browse the repository at this point in the history 
Also add missing ConfigureAwait(false) and StringComparison.Ordinal,
because of a configuration mismatch between FxCop and the built-in
diagnostics analyzers.

Fixes #26790
  • Loading branch information
@roji
roji authored Dec 16, 2021
1 parent 043f9d0 commit a778126
Show file tree
Hide file tree
Showing 13 changed files with 263 additions and 30 deletions.
2 changes: 1 addition & 1 deletion benchmark/Directory.Build.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<Import Project="..\Directory.Build.props" />

<PropertyGroup>
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)..\rulesets\EFCore.noxmldocs.ruleset</CodeAnalysisRuleSet>
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)..\rulesets\EFCore.test.ruleset</CodeAnalysisRuleSet>
</PropertyGroup>

<ItemGroup>
Expand Down
6 changes: 6 additions & 0 deletions rulesets/EFCore.test.ruleset
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="EFCore" ToolsVersion="15.0">
<Include Path="StyleCop.noxmldocs.ruleset" Action="Default" />
<Include Path="FxCop.test.ruleset" Action="Default" />
<Include Path="Documentation.ruleset" Action="Default" />
</RuleSet>
14 changes: 7 additions & 7 deletions rulesets/FxCop.ruleset
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
<Rule Id="CA1305" Action="None" /> <!-- Specify IFormatProvider -->
<Rule Id="CA1307" Action="None" /> <!-- Specify StringComparison -->
<Rule Id="CA1308" Action="None" /> <!-- Normalize strings to uppercase -->
<Rule Id="CA1309" Action="None" /> <!-- Use ordinal stringcomparison -->
<Rule Id="CA1309" Action="Error" /> <!-- Use ordinal stringcomparison -->
<Rule Id="CA1401" Action="None" /> <!-- P/Invokes should not be visible -->
<Rule Id="CA1501" Action="None" /> <!-- Avoid excessive inheritance -->
<Rule Id="CA1502" Action="None" /> <!-- Avoid excessive complexity -->
Expand Down Expand Up @@ -95,13 +95,13 @@
<Rule Id="CA1829" Action="None" /> <!-- Use Length/Count property instead of Count() when available -->
<Rule Id="CA2000" Action="None" /> <!-- Dispose objects before losing scope -->
<Rule Id="CA2002" Action="None" /> <!-- Do not lock on objects with weak identity -->
<Rule Id="CA2007" Action="None" /> <!-- Consider calling ConfigureAwait on the awaited task -->
<Rule Id="CA2008" Action="None" /> <!-- Do not create tasks without passing a TaskScheduler -->
<Rule Id="CA2009" Action="None" /> <!-- Do not call ToImmutableCollection on an ImmutableCollection value -->
<Rule Id="CA2007" Action="Error" /> <!-- Consider calling ConfigureAwait on the awaited task -->
<Rule Id="CA2008" Action="Error" /> <!-- Do not create tasks without passing a TaskScheduler -->
<Rule Id="CA2009" Action="Error" /> <!-- Do not call ToImmutableCollection on an ImmutableCollection value -->
<Rule Id="CA2010" Action="None" /> <!-- Always consume the value returned by methods marked with PreserveSigAttribute -->
<Rule Id="CA2011" Action="None" /> <!-- Avoid infinite recursion -->
<Rule Id="CA2012" Action="None" /> <!-- Use ValueTasks correctly -->
<Rule Id="CA2013" Action="None" /> <!-- Do not use ReferenceEquals with value types -->
<Rule Id="CA2011" Action="Error" /> <!-- Avoid infinite recursion -->
<Rule Id="CA2012" Action="Error" /> <!-- Use ValueTasks correctly -->
<Rule Id="CA2013" Action="Error" /> <!-- Do not use ReferenceEquals with value types -->
<Rule Id="CA2100" Action="None" /> <!-- Review SQL queries for security vulnerabilities -->
<Rule Id="CA2101" Action="None" /> <!-- Specify marshaling for P/Invoke string arguments -->
<Rule Id="CA2119" Action="None" /> <!-- Seal methods that satisfy private interfaces -->
Expand Down
218 changes: 218 additions & 0 deletions rulesets/FxCop.test.ruleset
View file

Large diffs are not rendered by default.

12 changes: 8 additions & 4 deletions src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,11 @@ private static async Task<bool> CreateItemOnceAsync(
(string ContainerId, JToken Document, IUpdateEntry Entry, CosmosClientWrapper Wrapper) parameters,
CancellationToken cancellationToken = default)
{
using var stream = new MemoryStream();
var stream = new MemoryStream();
await using var __ = stream.ConfigureAwait(false);
var writer = new StreamWriter(stream, new UTF8Encoding(), bufferSize: 1024, leaveOpen: false);
await using var __ = writer.ConfigureAwait(false);
await using var ___ = writer.ConfigureAwait(false);

using var jsonWriter = new JsonTextWriter(writer);
Serializer.Serialize(jsonWriter, parameters.Document);
await jsonWriter.FlushAsync(cancellationToken).ConfigureAwait(false);
Expand Down Expand Up @@ -311,8 +313,10 @@ private static async Task<bool> ReplaceItemOnceAsync(
(string ContainerId, string ResourceId, JObject Document, IUpdateEntry Entry, CosmosClientWrapper Wrapper) parameters,
CancellationToken cancellationToken = default)
{
await using var stream = new MemoryStream();
await using var writer = new StreamWriter(stream, new UTF8Encoding(), bufferSize: 1024, leaveOpen: false);
var stream = new MemoryStream();
await using var __ = stream.ConfigureAwait(false);
var writer = new StreamWriter(stream, new UTF8Encoding(), bufferSize: 1024, leaveOpen: false);
await using var ___ = writer.ConfigureAwait(false);
using var jsonWriter = new JsonTextWriter(writer);
Serializer.Serialize(jsonWriter, parameters.Document);
await jsonWriter.FlushAsync(cancellationToken).ConfigureAwait(false);
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3026,7 +3026,8 @@ private void AddTable(TableExpressionBase tableExpressionBase, TableReferenceExp
{
Check.DebugAssert(_tables.Count == _tableReferences.Count, "All the tables should have their associated TableReferences.");
Check.DebugAssert(
string.Equals(GetAliasFromTableExpressionBase(tableExpressionBase), tableReferenceExpression.Alias),
string.Equals(
GetAliasFromTableExpressionBase(tableExpressionBase), tableReferenceExpression.Alias, StringComparison.Ordinal),
"Alias of table and table reference should be the same.");

var uniqueAlias = GenerateUniqueAlias(_usedAliases, tableReferenceExpression.Alias);
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Storage/RelationalConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ public virtual bool Close()
}
else
{
CloseDbConnectionAsync();
CloseDbConnection();
}

wasClosed = true;
Expand Down
9 changes: 6 additions & 3 deletions src/EFCore.Relational/Storage/RelationalTransaction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,8 @@ public virtual async Task CreateSavepointAsync(string name, CancellationToken ca

if (!interceptionResult.IsSuppressed)
{
await using var command = Connection.DbConnection.CreateCommand();
var command = Connection.DbConnection.CreateCommand();
await using var _ = command.ConfigureAwait(false);
command.Transaction = _dbTransaction;
command.CommandText = _sqlGenerationHelper.GenerateCreateSavepointStatement(name);
await command.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false);
Expand Down Expand Up @@ -403,7 +404,8 @@ public virtual async Task RollbackToSavepointAsync(string name, CancellationToke

if (!interceptionResult.IsSuppressed)
{
await using var command = Connection.DbConnection.CreateCommand();
var command = Connection.DbConnection.CreateCommand();
await using var _ = command.ConfigureAwait(false);
command.Transaction = _dbTransaction;
command.CommandText = _sqlGenerationHelper.GenerateRollbackToSavepointStatement(name);
await command.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false);
Expand Down Expand Up @@ -492,7 +494,8 @@ public virtual async Task ReleaseSavepointAsync(string name, CancellationToken c

if (!interceptionResult.IsSuppressed)
{
await using var command = Connection.DbConnection.CreateCommand();
var command = Connection.DbConnection.CreateCommand();
await using var _ = command.ConfigureAwait(false);
command.Transaction = _dbTransaction;
command.CommandText = _sqlGenerationHelper.GenerateReleaseSavepointStatement(name);
await command.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false);
Expand Down
6 changes: 4 additions & 2 deletions src/EFCore.SqlServer/Storage/Internal/SqlServerDatabaseCreator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ public override void Create()
/// </summary>
public override async Task CreateAsync(CancellationToken cancellationToken = default)
{
await using (var masterConnection = _connection.CreateMasterConnection())
var masterConnection = _connection.CreateMasterConnection();
await using (masterConnection.ConfigureAwait(false))
{
await Dependencies.MigrationCommandExecutor
.ExecuteNonQueryAsync(CreateCreateOperations(), masterConnection, cancellationToken)
Expand Down Expand Up @@ -360,7 +361,8 @@ public override async Task DeleteAsync(CancellationToken cancellationToken = def
{
ClearAllPools();

await using var masterConnection = _connection.CreateMasterConnection();
var masterConnection = _connection.CreateMasterConnection();
await using var _ = masterConnection.ConfigureAwait(false);
await Dependencies.MigrationCommandExecutor
.ExecuteNonQueryAsync(CreateDropCommands(), masterConnection, cancellationToken)
.ConfigureAwait(false);
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/Metadata/Conventions/ForeignKeyAttributeConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ public virtual void ProcessEntityTypeAdded(

foreach (var inverse in inverses)
{
unconfiguredNavigations.RemoveAll(n => string.Equals(n.GetSimpleMemberName(), inverse));
foreignKeyNavigations.RemoveAll(n => string.Equals(n.GetSimpleMemberName(), inverse));
unconfiguredNavigations.RemoveAll(n => string.Equals(n.GetSimpleMemberName(), inverse, StringComparison.Ordinal));
foreignKeyNavigations.RemoveAll(n => string.Equals(n.GetSimpleMemberName(), inverse, StringComparison.Ordinal));
}

if (unconfiguredNavigations.Count == 1)
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Conventions/InversePropertyAttributeConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ private void Process(
var targetEntityType = targetEntityTypeBuilder.Metadata;
var targetClrType = targetEntityType.ClrType;
var inverseNavigationPropertyInfo = targetEntityType.GetRuntimeProperties().Values
.FirstOrDefault(p => string.Equals(p.GetSimpleMemberName(), attribute.Property))
.FirstOrDefault(p => string.Equals(p.GetSimpleMemberName(), attribute.Property, StringComparison.Ordinal))
?? targetEntityType.GetRuntimeProperties().Values
.FirstOrDefault(p => string.Equals(p.GetSimpleMemberName(), attribute.Property, StringComparison.OrdinalIgnoreCase));

Expand Down
13 changes: 6 additions & 7 deletions src/EFCore/Storage/ExecutionStrategyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -838,13 +838,12 @@ public static Task<TResult> ExecuteInTransactionAsync<TState, TResult>(
{
Check.NotNull(beginTransaction, nameof(beginTransaction));
var transaction = await beginTransaction(c, cancellationToken).ConfigureAwait(false);
await using (transaction)
{
s.CommitFailed = false;
s.Result = await s.Operation(s.State, ct).ConfigureAwait(false);
s.CommitFailed = true;
await transaction.CommitAsync(cancellationToken).ConfigureAwait(false);
}
await using var _ = transaction.ConfigureAwait(false);

s.CommitFailed = false;
s.Result = await s.Operation(s.State, ct).ConfigureAwait(false);
s.CommitFailed = true;
await transaction.CommitAsync(cancellationToken).ConfigureAwait(false);

return s.Result;
}, async (_, s, ct) => new ExecutionResult<TResult>(
Expand Down
2 changes: 1 addition & 1 deletion test/Directory.Build.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

<PropertyGroup>
<NoWarn>$(NoWarn);CA1707;1591;xUnit1000;xUnit1003;xUnit1004;xUnit1010;xUnit1013;xUnit1026</NoWarn>
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)..\rulesets\EFCore.noxmldocs.ruleset</CodeAnalysisRuleSet>
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)..\rulesets\EFCore.test.ruleset</CodeAnalysisRuleSet>
</PropertyGroup>

</Project>

0 comments on commit a778126

Please sign in to comment.