Skip to content

Commit d5dc241

Browse files
PartitionKey: Fixes latent issues in ContainerPropertiesExtensions and hardens InitTaskThreadSafe
Addresses PR Deep Reviewer findings surfaced while addressing Fabian's feedback: - GetItemIdFromStreamIfRequiredAsync: guards against ResolveByNameAsync returning a null ContainerProperties, matching the null guard already present in EnsureIdGetAppendedToPartitionKeyIfNeededAsync. Prevents an NRE on containerProperties.IsLastPartitionKeyPathId when the collection cache returns null. - Both helpers now rethrow OperationCanceledException instead of swallowing it in the blanket catch, so caller-cancelled tokens propagate as expected. - InitTaskThreadSafe: also resets metadataCallCount between iterations for hygiene (previously relied on VmMetadataApiHandler's static latch) and uses Interlocked.Exchange for counter resets to stay consistent with the increment side. Adds a comment explaining why the URI bag is drained in place rather than reassigned. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5a9a929 commit d5dc241

2 files changed

Lines changed: 14 additions & 3 deletions

File tree

Microsoft.Azure.Cosmos/src/Resource/Settings/ContainerPropertiesExtensions.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ internal static class ContainerPropertiesExtensions
3838
trace: NoOpTrace.Singleton,
3939
cancellationToken: cancellationToken);
4040
}
41+
catch (OperationCanceledException)
42+
{
43+
throw;
44+
}
4145
catch (Exception)
4246
{
4347
// Container may not exist yet (e.g. operations on non-existent containers).
@@ -115,14 +119,18 @@ internal static class ContainerPropertiesExtensions
115119
trace: NoOpTrace.Singleton,
116120
cancellationToken: cancellationToken);
117121
}
122+
catch (OperationCanceledException)
123+
{
124+
throw;
125+
}
118126
catch (Exception)
119127
{
120128
// Container may not exist yet (e.g. operations on non-existent containers).
121129
// Swallow the exception and let the actual operation surface the proper error.
122130
return (itemId, streamPayload);
123131
}
124132

125-
if (containerProperties.IsLastPartitionKeyPathId)
133+
if (containerProperties != null && containerProperties.IsLastPartitionKeyPathId)
126134
{
127135
return await ContainerPropertiesExtensions.GetIdFromStreamPayloadAsync(streamPayload);
128136
}

Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/ClientTests.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,14 @@ public async Task InitTaskThreadSafe()
157157
httpCallCount,
158158
$"Only the first task should do the http call. All other should wait on the first task. Observed URIs: [{string.Join(", ", httpRequestUris)}]");
159159

160-
// Reset counters and retry the client to verify a new http call is done for new requests
160+
// Reset counters and retry the client to verify a new http call is done for new requests.
161161
tasks.Clear();
162162
delayCallBack = true;
163163
this.TaskStartedCount = 0;
164-
httpCallCount = 0;
164+
Interlocked.Exchange(ref httpCallCount, 0);
165+
Interlocked.Exchange(ref metadataCallCount, 0);
166+
// Drain in-place: the RequestCallBack closure captured this specific bag instance,
167+
// so we cannot swap the reference without the handler writing into a stale bag.
165168
while (httpRequestUris.TryTake(out _))
166169
{
167170
}

0 commit comments

Comments
 (0)