From 704e42018e378dcd631f8205b7ed1fff12f109fa Mon Sep 17 00:00:00 2001 From: jeppesc11 Date: Sat, 27 Sep 2025 20:44:21 +0200 Subject: [PATCH 1/4] Preserving RequestId for NewBatchWithFailedRequests --- .../Requests/Content/BatchRequestContentCollection.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs b/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs index e7f441790..2e8ff3722 100644 --- a/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs +++ b/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs @@ -183,7 +183,13 @@ public BatchRequestContentCollection NewBatchWithFailedRequests(Dictionary Date: Sat, 27 Sep 2025 20:45:59 +0200 Subject: [PATCH 2/4] feat: Added statusCodesToTreatAsSuccess (additive success set) to NewBatchWithFailedRequests --- .../Content/BatchRequestContentCollection.cs | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs b/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs index 2e8ff3722..23f70bb37 100644 --- a/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs +++ b/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs @@ -194,5 +194,58 @@ public BatchRequestContentCollection NewBatchWithFailedRequests(Dictionary + /// Creates a new with all that failed. + /// + /// A dictionary with response codes, get by executing batchResponseContentCollection.GetResponsesStatusCodesAsync() + /// Optional additional HTTP status codes to also treat as successful. Success is determined by OR membership in this set + /// new with all failed requests. + public BatchRequestContentCollection NewBatchWithFailedRequests(Dictionary responseStatusCodes, IEnumerable statusCodesToTreatAsSuccess) + { + var request = new BatchRequestContentCollection(this.requestAdapter, batchRequestLimit); + if (responseStatusCodes == null || responseStatusCodes.Count == 0) + { + return request; + } + + HashSet successSet = statusCodesToTreatAsSuccess != null + ? [.. statusCodesToTreatAsSuccess] + : null; + + bool IsSuccess(HttpStatusCode code) + { + if (BatchResponseContent.IsSuccessStatusCode(code)) + { + return true; + } + + if (successSet != null && successSet.Contains(code)) + { + return true; + } + + return false; + } + + var steps = this.BatchRequestSteps; + foreach (var kvp in responseStatusCodes) + { + if (steps.TryGetValue(kvp.Key, out var step)) + { + if (!IsSuccess(kvp.Value)) + { + var newStep = new BatchRequestStep( + requestId: step.RequestId, + httpRequestMessage: step.Request, + dependsOn: step.DependsOn?.ToList()); + + request.AddBatchRequestStep(newStep); + } + } + } + + return request; + } } } From 1cbf0cda92f4ec0dafea467f91394f87322960b2 Mon Sep 17 00:00:00 2001 From: jeppesc11 Date: Sat, 27 Sep 2025 20:46:49 +0200 Subject: [PATCH 3/4] Added more test cases NewBatchWithFailedRequests --- .../Content/BatchRequestContentTests.cs | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs index 4ce0801fc..c5e5e87a6 100644 --- a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs +++ b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs @@ -692,5 +692,102 @@ public async Task BatchRequestContent_AddBatchRequestPutStepWithBaseRequestPrope // Assert we added successfully and contents are as expected and URI is not encoded Assert.Equal(expectedContent, System.Text.RegularExpressions.Regex.Unescape(requestContent)); } + + + [Fact] + public async Task BatchRequestContent_NewBatchWithFailedRequests_Files404_TreatedAsSuccessAsync() + { + var batchRequestContent = new BatchRequestContentCollection(client); + + var listChildren = new RequestInformation + { + HttpMethod = Method.GET, + UrlTemplate = "https://graph.microsoft.com/v1.0/me/drive/root/children" + }; + var listChildrenId = await batchRequestContent.AddBatchRequestStepAsync(listChildren); + + var getExistingFile = new RequestInformation + { + HttpMethod = Method.GET, + UrlTemplate = "https://graph.microsoft.com/v1.0/me/drive/items/existing" + }; + var okId = await batchRequestContent.AddBatchRequestStepAsync(getExistingFile); + + var responseStatusCodes = new Dictionary + { + { listChildrenId, HttpStatusCode.NotFound }, + { okId, HttpStatusCode.OK } + }; + + var successOverrides = new[] { HttpStatusCode.NotFound }; + + var retryBatch = batchRequestContent.NewBatchWithFailedRequests(responseStatusCodes, successOverrides); + + Assert.Empty(retryBatch.BatchRequestSteps); + } + + + [Fact] + public async Task BatchRequestContent_NewBatchWithFailedRequests_Files404_AndOk_MixesCorrectlyAsync() + { + var batchRequestContent = new BatchRequestContentCollection(client); + + var getMissingFile = new RequestInformation + { + HttpMethod = Method.GET, + UrlTemplate = "https://graph.microsoft.com/v1.0/me/drive/items/missing" + }; + var getExistingFile = new RequestInformation + { + HttpMethod = Method.GET, + UrlTemplate = "https://graph.microsoft.com/v1.0/me/drive/items/existing" + }; + + var missingId = await batchRequestContent.AddBatchRequestStepAsync(getMissingFile); + var okId = await batchRequestContent.AddBatchRequestStepAsync(getExistingFile); + + var responseStatusCodes = new Dictionary + { + { missingId, HttpStatusCode.NotFound }, + { okId, HttpStatusCode.OK } + }; + + var retryBatch = batchRequestContent.NewBatchWithFailedRequests(responseStatusCodes, null); + + Assert.Single(retryBatch.BatchRequestSteps); + Assert.True(retryBatch.BatchRequestSteps.ContainsKey(missingId)); + Assert.False(retryBatch.BatchRequestSteps.ContainsKey(okId)); + } + + [Fact] + public async Task BatchRequestContent_NewBatchWithFailedRequests_PreservesRequestIdsAsync() + { + var batchRequestContent = new BatchRequestContentCollection(client); + + var req1 = new RequestInformation { HttpMethod = Method.GET, UrlTemplate = REQUEST_URL }; + var req2 = new RequestInformation { HttpMethod = Method.GET, UrlTemplate = REQUEST_URL }; + var req3 = new RequestInformation { HttpMethod = Method.GET, UrlTemplate = REQUEST_URL }; + + var id1 = await batchRequestContent.AddBatchRequestStepAsync(req1); + var id2 = await batchRequestContent.AddBatchRequestStepAsync(req2); + var id3 = await batchRequestContent.AddBatchRequestStepAsync(req3); + + var responseStatusCodes = new Dictionary + { + { id1, HttpStatusCode.BadGateway }, + { id2, HttpStatusCode.OK }, + { id3, (HttpStatusCode)429 } + }; + + var retryBatch = batchRequestContent.NewBatchWithFailedRequests(responseStatusCodes); + + Assert.Equal(2, retryBatch.BatchRequestSteps.Count); + Assert.True(retryBatch.BatchRequestSteps.ContainsKey(id1)); + Assert.True(retryBatch.BatchRequestSteps.ContainsKey(id3)); + + Assert.Equal(id1, retryBatch.BatchRequestSteps[id1].RequestId); + Assert.Equal(id3, retryBatch.BatchRequestSteps[id3].RequestId); + } + } } From 7b8b1e4a7f183abfca89eb2d831a7152c84f7048 Mon Sep 17 00:00:00 2001 From: Jeppe Date: Mon, 10 Aug 2026 16:10:36 +0200 Subject: [PATCH 4/4] refactor: take HashSet for statusCodesToTreatAsSuccess per review Address review feedback: - NewBatchWithFailedRequests now requires the caller to pass a HashSet, removing the IEnumerable-to-HashSet conversion inside the method. - Collapse the IsSuccess local function into an expression body. Parentheses around the null-coalescing operand are required: without them `a || set?.Contains(code) ?? false` parses as `(a || bool?) ?? false` and fails to compile with CS0019. - The single-argument overload now delegates to the two-argument one so the RequestId-preserving step construction lives in one place. --- .../Content/BatchRequestContentCollection.cs | 55 ++++--------------- .../Content/BatchRequestContentTests.cs | 27 ++++++++- 2 files changed, 37 insertions(+), 45 deletions(-) diff --git a/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs b/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs index 23f70bb37..7c07d6988 100644 --- a/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs +++ b/src/Microsoft.Graph.Core/Requests/Content/BatchRequestContentCollection.cs @@ -177,22 +177,7 @@ public IReadOnlyDictionary BatchRequestSteps /// new with all failed requests. public BatchRequestContentCollection NewBatchWithFailedRequests(Dictionary responseStatusCodes) { - var request = new BatchRequestContentCollection(this.requestAdapter, batchRequestLimit); - var steps = this.BatchRequestSteps; - foreach (var response in responseStatusCodes) - { - if (steps.ContainsKey(response.Key) && !BatchResponseContent.IsSuccessStatusCode(response.Value)) - { - var step = steps[response.Key]; - var newStep = new BatchRequestStep( - requestId: step.RequestId, - httpRequestMessage: step.Request, - dependsOn: step.DependsOn?.ToList()); - - request.AddBatchRequestStep(newStep); - } - } - return request; + return NewBatchWithFailedRequests(responseStatusCodes, null); } /// @@ -201,7 +186,7 @@ public BatchRequestContentCollection NewBatchWithFailedRequests(DictionaryA dictionary with response codes, get by executing batchResponseContentCollection.GetResponsesStatusCodesAsync() /// Optional additional HTTP status codes to also treat as successful. Success is determined by OR membership in this set /// new with all failed requests. - public BatchRequestContentCollection NewBatchWithFailedRequests(Dictionary responseStatusCodes, IEnumerable statusCodesToTreatAsSuccess) + public BatchRequestContentCollection NewBatchWithFailedRequests(Dictionary responseStatusCodes, HashSet statusCodesToTreatAsSuccess) { var request = new BatchRequestContentCollection(this.requestAdapter, batchRequestLimit); if (responseStatusCodes == null || responseStatusCodes.Count == 0) @@ -209,39 +194,21 @@ public BatchRequestContentCollection NewBatchWithFailedRequests(Dictionary successSet = statusCodesToTreatAsSuccess != null - ? [.. statusCodesToTreatAsSuccess] - : null; - - bool IsSuccess(HttpStatusCode code) - { - if (BatchResponseContent.IsSuccessStatusCode(code)) - { - return true; - } - - if (successSet != null && successSet.Contains(code)) - { - return true; - } - - return false; - } + bool IsSuccess(HttpStatusCode code) => + BatchResponseContent.IsSuccessStatusCode(code) + || (statusCodesToTreatAsSuccess?.Contains(code) ?? false); var steps = this.BatchRequestSteps; foreach (var kvp in responseStatusCodes) { - if (steps.TryGetValue(kvp.Key, out var step)) + if (steps.TryGetValue(kvp.Key, out var step) && !IsSuccess(kvp.Value)) { - if (!IsSuccess(kvp.Value)) - { - var newStep = new BatchRequestStep( - requestId: step.RequestId, - httpRequestMessage: step.Request, - dependsOn: step.DependsOn?.ToList()); + var newStep = new BatchRequestStep( + requestId: step.RequestId, + httpRequestMessage: step.Request, + dependsOn: step.DependsOn?.ToList()); - request.AddBatchRequestStep(newStep); - } + request.AddBatchRequestStep(newStep); } } diff --git a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs index c5e5e87a6..047bdd3d9 100644 --- a/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs +++ b/tests/Microsoft.Graph.DotnetCore.Core.Test/Requests/Content/BatchRequestContentTests.cs @@ -719,7 +719,7 @@ public async Task BatchRequestContent_NewBatchWithFailedRequests_Files404_Treate { okId, HttpStatusCode.OK } }; - var successOverrides = new[] { HttpStatusCode.NotFound }; + var successOverrides = new HashSet { HttpStatusCode.NotFound }; var retryBatch = batchRequestContent.NewBatchWithFailedRequests(responseStatusCodes, successOverrides); @@ -789,5 +789,30 @@ public async Task BatchRequestContent_NewBatchWithFailedRequests_PreservesReques Assert.Equal(id3, retryBatch.BatchRequestSteps[id3].RequestId); } + [Fact] + public async Task BatchRequestContent_NewBatchWithFailedRequests_NullSuccessSetMatchesSingleArgOverloadAsync() + { + var batchRequestContent = new BatchRequestContentCollection(client); + + var req1 = new RequestInformation { HttpMethod = Method.GET, UrlTemplate = REQUEST_URL }; + var req2 = new RequestInformation { HttpMethod = Method.GET, UrlTemplate = REQUEST_URL }; + + var failedId = await batchRequestContent.AddBatchRequestStepAsync(req1); + var okId = await batchRequestContent.AddBatchRequestStepAsync(req2); + + var responseStatusCodes = new Dictionary + { + { failedId, HttpStatusCode.BadGateway }, + { okId, HttpStatusCode.OK } + }; + + var singleArgBatch = batchRequestContent.NewBatchWithFailedRequests(responseStatusCodes); + var nullSetBatch = batchRequestContent.NewBatchWithFailedRequests(responseStatusCodes, null); + + Assert.Equal(singleArgBatch.BatchRequestSteps.Keys.OrderBy(key => key), nullSetBatch.BatchRequestSteps.Keys.OrderBy(key => key)); + Assert.Single(nullSetBatch.BatchRequestSteps); + Assert.Equal(failedId, nullSetBatch.BatchRequestSteps[failedId].RequestId); + } + } }