Repository navigation
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the Prepare method in SpannerCommand to throw an InvalidOperationException if the connection is unset or not open, and adds corresponding unit tests. The review feedback suggests caching the SpannerConnection property in a local variable before checking its state to ensure thread safety and avoid potential compiler warnings.
| if (SpannerConnection == null) | ||
| { | ||
| throw new InvalidOperationException("The Connection property has not been set."); | ||
| } | ||
| if (SpannerConnection.State != ConnectionState.Open) | ||
| { | ||
| throw new InvalidOperationException("The connection is not open."); | ||
| } |
There was a problem hiding this comment.
To ensure thread safety and avoid potential compiler warnings (such as CS8602) when nullable reference types are enabled, it is recommended to cache the mutable SpannerConnection property in a local variable before performing the null check and accessing its State property.
var connection = SpannerConnection;
if (connection == null)
{
throw new InvalidOperationException("The Connection property has not been set.");
}
if (connection.State != ConnectionState.Open)
{
throw new InvalidOperationException("The connection is not open.");
}686dda4 to
b5ce074
Compare
b5ce074 to
bb3758d
Compare
Summary
Make
SpannerCommand.PreparethrowInvalidOperationExceptionwhen no connection is assigned or when its connection is closed. Keep preparation a no-op when the connection is open, as Spanner does not support precompiled queries. Add regression tests for both invalid states.Resolves #16047.
Microsoft contract
Microsoft documents
InvalidOperationExceptionwhenIDbCommand.Prepareis called without a connection or with a closed connection: https://learn.microsoft.com/dotnet/api/system.data.idbcommand.prepareCompatibility
Commands with an open connection retain the existing no-op preparation behavior. Calls that currently succeed without an open connection will now throw as documented; applications relying on preparing disconnected commands may observe this change.
Validation
Focused provider tests:
dotnet test apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.Tests/Google.Cloud.Spanner.Data.Tests.csproj --no-restore -f net8.0 -p:NoWarn=CS8981%3BxUnit2004%3BxUnit2013 --filter FullyQualifiedName~Prepare— 2 passed, 0 failed.Discovery
Found using pengdows.AdoNetApiTest.conformance.