Skip to content

Commit 25bc8ed

Browse files
committed
chore: Resolve Gemini review comments
1 parent 0ee1b7e commit 25bc8ed

File tree

6 files changed

+13
-44
lines changed

6 files changed

+13
-44
lines changed

apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.IntegrationTests/V1/ManagedSessionTests.cs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ public async Task RunReadWriteTransactionWithMultipleQueries()
7777
Assert.NotNull(transaction.TransactionId);
7878
Assert.True(transaction.PrecommitToken.SeqNum >= preCommitTokenSeqNumber);
7979

80-
preCommitTokenSeqNumber = transaction.PrecommitToken.SeqNum;
81-
8280
// Commit the transaction
8381
var commitResponse = await transaction.CommitAsync(new CommitRequest(), null);
8482
Assert.NotNull(commitResponse);
@@ -131,12 +129,10 @@ private async Task IncrementByOneAsync(ManagedTransaction transaction, string un
131129
retryFilter: ignored => false,
132130
RetrySettings.RandomJitter);
133131
TimeSpan nextDelay = TimeSpan.Zero;
134-
SpannerException spannerException;
135132
DateTime deadline = DateTime.UtcNow.AddSeconds(30);
136133

137134
while (true)
138135
{
139-
spannerException = null;
140136
try
141137
{
142138
// We use manually created transactions here so the tests run on .NET Core.
@@ -154,14 +150,13 @@ private async Task IncrementByOneAsync(ManagedTransaction transaction, string un
154150
}
155151

156152

157-
ResultSet result;
158153
if (current == 0)
159154
{
160-
result = await ExecuteInsertInt64Value(transaction, uniqueRowId, current + 1);
155+
await ExecuteInsertInt64Value(transaction, uniqueRowId, current + 1);
161156
}
162157
else
163158
{
164-
result = await ExecuteUpdateInt64Value(transaction, uniqueRowId, current + 1);
159+
await ExecuteUpdateInt64Value(transaction, uniqueRowId, current + 1);
165160
}
166161

167162
await transaction.CommitAsync(new CommitRequest(), null);
@@ -172,7 +167,6 @@ private async Task IncrementByOneAsync(ManagedTransaction transaction, string un
172167
{
173168
nextDelay = retrySettings.NextBackoff(nextDelay);
174169
await Task.Delay(retrySettings.BackoffJitter.GetDelay(nextDelay));
175-
spannerException = ex;
176170
}
177171
}
178172
}

apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.Tests/V1/DirectedReadTests.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,6 @@ public async Task PooledSession_NonReadOnlyTransaction_IgnoresOptionsFromClient_
325325
}, logger: null);
326326

327327
ManagedTransaction managedTransaction = await CreateManagedTransaction(spannerClient, s_transactionId, options, false);
328-
//managedTransaction = managedTransaction.WithTransaction(s_transactionId, options, singleUseTransaction: false);
329328

330329
await managedTransaction.ReadStreamReaderAsync(new ReadRequest(), callSettings: null).HasDataAsync(default);
331330
Assert.Null(grpcClient.LastReadRequest.DirectedReadOptions);

apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data/SessionPoolManager.cs

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -164,18 +164,7 @@ internal Task<ManagedSession> AcquireManagedSessionAsync(SpannerClientCreationOp
164164
SessionPoolSegmentKey segmentKey = SessionPoolSegmentKey.Create(dbName).WithDatabaseRole(dbRole);
165165
GaxPreconditions.CheckNotNull(options, nameof(options));
166166

167-
if (_targetedSessions.ContainsKey((options, segmentKey)))
168-
{
169-
return _targetedSessions[(options, segmentKey)];
170-
}
171-
else
172-
{
173-
_targetedSessions[(options, segmentKey)] = CreateMultiplexSessionAsync();
174-
}
175-
176-
var muxSession = _targetedSessions[(options, segmentKey)];
177-
178-
return muxSession;
167+
return _targetedSessions.GetOrAdd((options, segmentKey), _ => CreateMultiplexSessionAsync());
179168

180169
async Task<ManagedSession> CreateMultiplexSessionAsync()
181170
{

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,9 @@ Task TriggerRefresh(double refreshInterval)
247247
if (SessionHasExpired(refreshInterval))
248248
{
249249
// Create/Refresh the session only if it is null or it is older than the refreshInterval
250-
multiplexSession = await Client.CreateSessionAsync(_createSessionRequestTemplate, callSettings).ConfigureAwait(false);
250+
multiplexSession = await Client.CreateSessionAsync(_createSessionRequestTemplate, callSettings)
251+
.WithCancellationToken(cancellationToken).ConfigureAwait(false);
252+
251253
Interlocked.Exchange(ref _session, multiplexSession);
252254
}
253255
}
@@ -296,12 +298,12 @@ Task TriggerRefresh(double refreshInterval)
296298
}
297299

298300
/// <summary>
299-
///
301+
/// Builder to build a <see cref="ManagedSession"/>
300302
/// </summary>
301303
public sealed partial class SessionBuilder
302304
{
303305
/// <summary>
304-
///
306+
/// Constructor with validations on essential parameters needed to build the ManagedSession
305307
/// </summary>
306308
public SessionBuilder(DatabaseName databaseName, SpannerClient client)
307309
{
@@ -330,9 +332,9 @@ public SessionBuilder(DatabaseName databaseName, SpannerClient client)
330332
public SpannerClient Client { get; set; }
331333

332334
/// <summary>
333-
///
335+
/// Async method to build a managed session. This will fetch a valid session from backend Spanner and wrap it inside the managed session object.
334336
/// </summary>
335-
/// <param name="cancellationToken"></param>
337+
/// <param name="cancellationToken">Cancellation token to cancel this call</param>
336338
/// <returns></returns>
337339
public async Task<ManagedSession> BuildAsync(CancellationToken cancellationToken = default)
338340
{

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSessionOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
namespace Google.Cloud.Spanner.V1;
1818

1919
/// <summary>
20-
///
20+
/// Options which can be set on the managed session
2121
/// </summary>
2222
public class ManagedSessionOptions
2323
{

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
namespace Google.Cloud.Spanner.V1
2727
{
2828
/// <summary>
29-
///
29+
/// Class which manages an underlying Spanner Transaction and operations around it
3030
/// </summary>
3131
public partial class ManagedTransaction
3232
{
@@ -440,23 +440,8 @@ Mutation MaybeFetchMutationKey(Protobuf.Collections.RepeatedField<Mutation> muta
440440
// 2. It is a Delete mutation AND Keyset.Keys.Count >= 1
441441

442442
Mutation filteredMutationKey = mutations.FirstOrDefault(mutation =>
443-
{
444-
// Condition 1: Check if it's NOT a Delete mutation
445-
bool isNotDelete = mutation.Delete == null;
446-
447-
if (isNotDelete)
448-
{
449-
return true;
450-
}
451-
452-
// The mutation MUST be a Delete mutation if we reach this point.
453-
// Condition 2: Check if it's a Delete mutation AND the keyset is not empty
454-
bool isDeleteWithNonEmptyKeySet = mutation.Delete != null &&
455-
mutation.Delete.KeySet != null &&
456-
mutation.Delete.KeySet.Keys.Count >= 1;
443+
mutation.Delete is null || (mutation.Delete.KeySet?.Keys.Count ?? 0) >= 1);
457444

458-
return isDeleteWithNonEmptyKeySet;
459-
});
460445

461446
return filteredMutationKey;
462447
}

0 commit comments

Comments
 (0)