Skip to content

Commit 3159357

Browse files
kirankumarkolliCopilot
kirankumarkolli
and
Copilot
committed
RefreshReason Phase 4: harden test coverage
- Factor FromTransportErrorCode into an internal TryMapKnownTransportErrorCode helper that returns false for unhandled codes. The public API still falls back to GoneUnknown for unknown codes (graceful in production), but the exhaustive unit test now uses TryMap directly, so any new upstream TransportErrorCode will fail CI instead of silently folding to GoneUnknown. - Promote GatewayAddressCache.EmitRefreshReasonHeader from private to internal so the validator/precedence behavior can be unit-tested without a full gateway round-trip. - Add 7 ClassifyGoneFromException tests covering every SubStatusCodes branch (CompletingSplit, CompletingPartitionMigration, NameCacheIsStale, PartitionKeyRangeGone) and the default GoneServer fallback. - Add RefreshReasonEmissionTests with 6 tests for precedence (explicit > ctx), null-request path (on-demand unhealthy-URI refresh), default no-op when untagged, and the opt-in ValidateRefreshReasonPresence invariant that guards future force-refresh call sites from shipping untagged. Tests: 25/25 Routing/RefreshReason pass; 22/22 GatewayAddressCache pass. Build: clean (0 warn / 0 err). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1c82164 commit 3159357

4 files changed

Lines changed: 311 additions & 21 deletions

File tree

‎Microsoft.Azure.Cosmos/src/Routing/GatewayAddressCache.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ await this.GetServerAddressesViaGatewayAsync(request, collectionRid, new[] { par
742742
/// throws <see cref="InvalidOperationException"/> so that any untagged
743743
/// force-refresh site is caught in tests.
744744
/// </summary>
745-
private static void EmitRefreshReasonHeader(
745+
internal static void EmitRefreshReasonHeader(
746746
INameValueCollection headers,
747747
DocumentServiceRequest request,
748748
RefreshReason explicitReason,

‎Microsoft.Azure.Cosmos/src/Routing/RefreshReasonExtensions.cs‎

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,59 +86,89 @@ public static string ToHeaderValue(this RefreshReason reason)
8686
/// <see cref="RefreshReason.GoneUnknown"/>.
8787
/// </summary>
8888
public static RefreshReason FromTransportErrorCode(TransportErrorCode code)
89+
{
90+
if (TryMapKnownTransportErrorCode(code, out RefreshReason reason))
91+
{
92+
return reason;
93+
}
94+
95+
// A new TransportErrorCode was added upstream without updating
96+
// the switch below. Fall back to GoneUnknown so the gateway still
97+
// gets *a* reason; the exhaustive test in
98+
// RefreshReasonFormatterTests will fail in CI and prompt a fix.
99+
return RefreshReason.GoneUnknown;
100+
}
101+
102+
/// <summary>
103+
/// Explicit-case mapping for <see cref="TransportErrorCode"/>. Returns
104+
/// <c>false</c> for codes that are not explicitly handled — this is
105+
/// how the exhaustive test detects new upstream codes (a silent
106+
/// fall-through to <see cref="RefreshReason.GoneUnknown"/> in the
107+
/// public <see cref="FromTransportErrorCode"/> would hide them).
108+
/// </summary>
109+
internal static bool TryMapKnownTransportErrorCode(TransportErrorCode code, out RefreshReason reason)
89110
{
90111
switch (code)
91112
{
92113
case TransportErrorCode.Unknown:
93114
case TransportErrorCode.ChannelOpenFailed:
94115
case TransportErrorCode.ChannelOpenTimeout:
95116
case TransportErrorCode.RequestTimeout:
96-
return RefreshReason.GoneUnknown;
117+
reason = RefreshReason.GoneUnknown;
118+
return true;
97119

98120
case TransportErrorCode.DnsResolutionFailed:
99121
case TransportErrorCode.DnsResolutionTimeout:
100-
return RefreshReason.GoneDnsResolution;
122+
reason = RefreshReason.GoneDnsResolution;
123+
return true;
101124

102125
case TransportErrorCode.ConnectFailed:
103126
case TransportErrorCode.ConnectTimeout:
104-
return RefreshReason.GoneConnect;
127+
reason = RefreshReason.GoneConnect;
128+
return true;
105129

106130
case TransportErrorCode.SslNegotiationFailed:
107131
case TransportErrorCode.SslNegotiationTimeout:
108-
return RefreshReason.GoneSslNegotiation;
132+
reason = RefreshReason.GoneSslNegotiation;
133+
return true;
109134

110135
case TransportErrorCode.TransportNegotiationTimeout:
111-
return RefreshReason.GoneNegotiationTimeout;
136+
reason = RefreshReason.GoneNegotiationTimeout;
137+
return true;
112138

113139
case TransportErrorCode.ChannelMultiplexerClosed:
114-
return RefreshReason.GoneChannelMultiplexerClosed;
140+
reason = RefreshReason.GoneChannelMultiplexerClosed;
141+
return true;
115142

116143
case TransportErrorCode.SendFailed:
117144
case TransportErrorCode.SendTimeout:
118-
return RefreshReason.GoneSend;
145+
reason = RefreshReason.GoneSend;
146+
return true;
119147

120148
case TransportErrorCode.SendLockTimeout:
121-
return RefreshReason.GoneSendLockTimeout;
149+
reason = RefreshReason.GoneSendLockTimeout;
150+
return true;
122151

123152
case TransportErrorCode.ReceiveFailed:
124153
case TransportErrorCode.ReceiveTimeout:
125-
return RefreshReason.GoneReceive;
154+
reason = RefreshReason.GoneReceive;
155+
return true;
126156

127157
case TransportErrorCode.ReceiveStreamClosed:
128-
return RefreshReason.GoneReceiveStreamClosed;
158+
reason = RefreshReason.GoneReceiveStreamClosed;
159+
return true;
129160

130161
case TransportErrorCode.ConnectionBroken:
131-
return RefreshReason.GoneConnectionBroken;
162+
reason = RefreshReason.GoneConnectionBroken;
163+
return true;
132164

133165
case TransportErrorCode.ChannelWaitingToOpenTimeout:
134-
return RefreshReason.GoneChannelWaitingToOpenTimeout;
166+
reason = RefreshReason.GoneChannelWaitingToOpenTimeout;
167+
return true;
135168

136169
default:
137-
// A new TransportErrorCode was added upstream without
138-
// updating this switch. Fall back to GoneUnknown so the
139-
// gateway still gets *a* reason; the exhaustive test in
140-
// RefreshReasonFormatterTests will fail and prompt a fix.
141-
return RefreshReason.GoneUnknown;
170+
reason = RefreshReason.Unspecified;
171+
return false;
142172
}
143173
}
144174

‎Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Routing/RefreshReasonEmissionTests.cs‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
//------------------------------------------------------------
2+
// Copyright (c) Microsoft Corporation. All rights reserved.
3+
//------------------------------------------------------------
4+
5+
namespace Microsoft.Azure.Cosmos.Tests.Routing
6+
{
7+
using System;
8+
using Microsoft.Azure.Cosmos.Routing;
9+
using Microsoft.Azure.Documents;
10+
using Microsoft.Azure.Documents.Collections;
11+
using Microsoft.VisualStudio.TestTools.UnitTesting;
12+
13+
/// <summary>
14+
/// Tests for <see cref="GatewayAddressCache.EmitRefreshReasonHeader"/>:
15+
/// verifies precedence (explicit &gt; request context &gt; default), header
16+
/// emission, and the opt-in validator invariant that guards untagged
17+
/// force-refresh sites.
18+
/// </summary>
19+
[TestClass]
20+
public class RefreshReasonEmissionTests
21+
{
22+
[TestMethod]
23+
public void Emit_ExplicitReason_WinsOverRequestContext()
24+
{
25+
INameValueCollection headers = new RequestNameValueCollection();
26+
using DocumentServiceRequest request = DocumentServiceRequest.Create(
27+
OperationType.Read,
28+
ResourceType.Document,
29+
AuthorizationTokenType.PrimaryMasterKey);
30+
request.RequestContext.RefreshReason = RefreshReason.GoneServer;
31+
32+
GatewayAddressCache.EmitRefreshReasonHeader(
33+
headers: headers,
34+
request: request,
35+
explicitReason: RefreshReason.ReplicaHealthUnhealthyLongLived,
36+
callerName: nameof(this.Emit_ExplicitReason_WinsOverRequestContext));
37+
38+
Assert.AreEqual(
39+
"ReplicaHealth.unhealthyLongLived",
40+
headers.Get(HttpConstants.HttpHeaders.CosmosRefreshReason));
41+
}
42+
43+
[TestMethod]
44+
public void Emit_NoExplicit_UsesRequestContextReason()
45+
{
46+
INameValueCollection headers = new RequestNameValueCollection();
47+
using DocumentServiceRequest request = DocumentServiceRequest.Create(
48+
OperationType.Read,
49+
ResourceType.Document,
50+
AuthorizationTokenType.PrimaryMasterKey);
51+
request.RequestContext.RefreshReason = RefreshReason.GoneConnect;
52+
53+
GatewayAddressCache.EmitRefreshReasonHeader(
54+
headers: headers,
55+
request: request,
56+
explicitReason: RefreshReason.Unspecified,
57+
callerName: nameof(this.Emit_NoExplicit_UsesRequestContextReason));
58+
59+
Assert.AreEqual(
60+
"gone.connect",
61+
headers.Get(HttpConstants.HttpHeaders.CosmosRefreshReason));
62+
}
63+
64+
[TestMethod]
65+
public void Emit_NullRequest_ExplicitReasonWrites()
66+
{
67+
// The on-demand unhealthy-URI refresh path calls with request=null.
68+
INameValueCollection headers = new RequestNameValueCollection();
69+
70+
GatewayAddressCache.EmitRefreshReasonHeader(
71+
headers: headers,
72+
request: null,
73+
explicitReason: RefreshReason.ReplicaHealthUnhealthyLongLived,
74+
callerName: nameof(this.Emit_NullRequest_ExplicitReasonWrites));
75+
76+
Assert.AreEqual(
77+
"ReplicaHealth.unhealthyLongLived",
78+
headers.Get(HttpConstants.HttpHeaders.CosmosRefreshReason));
79+
}
80+
81+
[TestMethod]
82+
public void Emit_BothUnspecified_ValidatorOff_NoHeader()
83+
{
84+
// Default behavior: when nothing is tagged, no header is written
85+
// (zero production overhead, graceful degradation).
86+
bool previous = GatewayAddressCache.ValidateRefreshReasonPresence;
87+
try
88+
{
89+
GatewayAddressCache.ValidateRefreshReasonPresence = false;
90+
INameValueCollection headers = new RequestNameValueCollection();
91+
using DocumentServiceRequest request = DocumentServiceRequest.Create(
92+
OperationType.Read,
93+
ResourceType.Document,
94+
AuthorizationTokenType.PrimaryMasterKey);
95+
96+
GatewayAddressCache.EmitRefreshReasonHeader(
97+
headers: headers,
98+
request: request,
99+
explicitReason: RefreshReason.Unspecified,
100+
callerName: nameof(this.Emit_BothUnspecified_ValidatorOff_NoHeader));
101+
102+
Assert.IsNull(headers.Get(HttpConstants.HttpHeaders.CosmosRefreshReason));
103+
}
104+
finally
105+
{
106+
GatewayAddressCache.ValidateRefreshReasonPresence = previous;
107+
}
108+
}
109+
110+
[TestMethod]
111+
public void Emit_BothUnspecified_ValidatorOn_Throws()
112+
{
113+
// Opt-in invariant: any forced address-cache refresh without a
114+
// tagged reason must throw, so new untagged call sites are caught
115+
// automatically in CI.
116+
bool previous = GatewayAddressCache.ValidateRefreshReasonPresence;
117+
try
118+
{
119+
GatewayAddressCache.ValidateRefreshReasonPresence = true;
120+
INameValueCollection headers = new RequestNameValueCollection();
121+
using DocumentServiceRequest request = DocumentServiceRequest.Create(
122+
OperationType.Read,
123+
ResourceType.Document,
124+
AuthorizationTokenType.PrimaryMasterKey);
125+
126+
InvalidOperationException ex = Assert.ThrowsException<InvalidOperationException>(
127+
() => GatewayAddressCache.EmitRefreshReasonHeader(
128+
headers: headers,
129+
request: request,
130+
explicitReason: RefreshReason.Unspecified,
131+
callerName: "TestCaller"));
132+
133+
StringAssert.Contains(ex.Message, "TestCaller");
134+
StringAssert.Contains(ex.Message, "RefreshReason");
135+
}
136+
finally
137+
{
138+
GatewayAddressCache.ValidateRefreshReasonPresence = previous;
139+
}
140+
}
141+
142+
[TestMethod]
143+
public void Emit_ValidatorOn_ExplicitReasonSet_DoesNotThrow()
144+
{
145+
bool previous = GatewayAddressCache.ValidateRefreshReasonPresence;
146+
try
147+
{
148+
GatewayAddressCache.ValidateRefreshReasonPresence = true;
149+
INameValueCollection headers = new RequestNameValueCollection();
150+
using DocumentServiceRequest request = DocumentServiceRequest.Create(
151+
OperationType.Read,
152+
ResourceType.Document,
153+
AuthorizationTokenType.PrimaryMasterKey);
154+
155+
GatewayAddressCache.EmitRefreshReasonHeader(
156+
headers: headers,
157+
request: request,
158+
explicitReason: RefreshReason.InsufficientReplicasSuboptimalTimer,
159+
callerName: nameof(this.Emit_ValidatorOn_ExplicitReasonSet_DoesNotThrow));
160+
161+
Assert.AreEqual(
162+
"InsufficientReplicas.SuboptimalTimer",
163+
headers.Get(HttpConstants.HttpHeaders.CosmosRefreshReason));
164+
}
165+
finally
166+
{
167+
GatewayAddressCache.ValidateRefreshReasonPresence = previous;
168+
}
169+
}
170+
}
171+
}

‎Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Routing/RefreshReasonFormatterTests.cs‎

Lines changed: 92 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,32 @@ public void ToHeaderValue_UnknownEnumValue_Throws()
8787

8888
/// <summary>
8989
/// Exhaustive coverage: every declared <see cref="TransportErrorCode"/>
90-
/// must map to a non-<see cref="RefreshReason.Unspecified"/> value.
91-
/// This fails (in CI) if an upstream TransportErrorCode is added
92-
/// without updating the switch in FromTransportErrorCode.
90+
/// must be explicitly handled by
91+
/// <c>RefreshReasonExtensions.TryMapKnownTransportErrorCode</c>. This
92+
/// test fails in CI if an upstream code is added without updating the
93+
/// switch (without this, new codes would silently fall through to
94+
/// <see cref="RefreshReason.GoneUnknown"/> in the public API).
9395
/// </summary>
96+
[TestMethod]
97+
public void TryMapKnownTransportErrorCode_CoversEveryCode()
98+
{
99+
List<TransportErrorCode> unhandled = new List<TransportErrorCode>();
100+
foreach (TransportErrorCode code in Enum.GetValues(typeof(TransportErrorCode)))
101+
{
102+
if (!RefreshReasonExtensions.TryMapKnownTransportErrorCode(code, out RefreshReason _))
103+
{
104+
unhandled.Add(code);
105+
}
106+
}
107+
108+
Assert.AreEqual(
109+
0,
110+
unhandled.Count,
111+
"TransportErrorCode(s) not explicitly handled by TryMapKnownTransportErrorCode: " +
112+
string.Join(", ", unhandled) +
113+
". Add explicit cases in RefreshReasonExtensions.");
114+
}
115+
94116
[TestMethod]
95117
public void FromTransportErrorCode_CoversEveryCode()
96118
{
@@ -178,5 +200,72 @@ public void EnumCount_MatchesWireValuesCount()
178200
RefreshReasonExtensions.WireValues.Count,
179201
"Every RefreshReason member must have exactly one entry in WireValues (no orphans, no extras).");
180202
}
203+
204+
// ---- ClassifyGoneFromException -------------------------------------
205+
// Transport-exception walking is covered transitively by the exhaustive
206+
// FromTransportErrorCode tests above; constructing TransportException
207+
// instances directly in unit tests hits a resource-manifest path that
208+
// isn't resolvable in the test host. The substatus paths below cover
209+
// the switch-case behavior fully.
210+
211+
[TestMethod]
212+
public void ClassifyGoneFromException_NoTransport_SubStatusCompletingSplit_MapsToSplit()
213+
{
214+
Assert.AreEqual(
215+
RefreshReason.GoneCompletingSplit,
216+
RefreshReasonExtensions.ClassifyGoneFromException(
217+
exception: null,
218+
subStatusCode: SubStatusCodes.CompletingSplit));
219+
}
220+
221+
[TestMethod]
222+
public void ClassifyGoneFromException_NoTransport_SubStatusCompletingPartitionMigration_MapsToMigration()
223+
{
224+
Assert.AreEqual(
225+
RefreshReason.GoneCompletingPartitionMigration,
226+
RefreshReasonExtensions.ClassifyGoneFromException(
227+
exception: null,
228+
subStatusCode: SubStatusCodes.CompletingPartitionMigration));
229+
}
230+
231+
[TestMethod]
232+
public void ClassifyGoneFromException_NoTransport_SubStatusNameCacheIsStale_MapsToNameCacheStale()
233+
{
234+
Assert.AreEqual(
235+
RefreshReason.GoneNameCacheStale,
236+
RefreshReasonExtensions.ClassifyGoneFromException(
237+
exception: null,
238+
subStatusCode: SubStatusCodes.NameCacheIsStale));
239+
}
240+
241+
[TestMethod]
242+
public void ClassifyGoneFromException_NoTransport_SubStatusPartitionKeyRangeGone_MapsToPkrGone()
243+
{
244+
Assert.AreEqual(
245+
RefreshReason.GonePartitionKeyRangeGone,
246+
RefreshReasonExtensions.ClassifyGoneFromException(
247+
exception: null,
248+
subStatusCode: SubStatusCodes.PartitionKeyRangeGone));
249+
}
250+
251+
[TestMethod]
252+
public void ClassifyGoneFromException_NoTransport_PlainGone_MapsToGoneServer()
253+
{
254+
Assert.AreEqual(
255+
RefreshReason.GoneServer,
256+
RefreshReasonExtensions.ClassifyGoneFromException(
257+
exception: new Exception("server 410"),
258+
subStatusCode: SubStatusCodes.Unknown));
259+
}
260+
261+
[TestMethod]
262+
public void ClassifyGoneFromException_NullException_NoSubStatus_MapsToGoneServer()
263+
{
264+
Assert.AreEqual(
265+
RefreshReason.GoneServer,
266+
RefreshReasonExtensions.ClassifyGoneFromException(
267+
exception: null,
268+
subStatusCode: SubStatusCodes.Unknown));
269+
}
181270
}
182271
}

0 commit comments

Comments
 (0)