From 7f918db9e5daa62d20accb02fc8f0acda0faf80f Mon Sep 17 00:00:00 2001 From: Scott Beddall <45376673+scbedd@users.noreply.github.com> Date: Sun, 4 Aug 2024 12:40:40 -0700 Subject: [PATCH] Allow already-removed sanitizers to not error when removed (#8770) * allow the sanitizers to clear * remove a test that doesn't have a point if we don't error on sanitizer removal --- .../AdminTests.cs | 34 ++----------------- .../Azure.Sdk.Tools.TestProxy/Admin.cs | 25 ++++---------- .../Common/SanitizerDictionary.cs | 4 +-- 3 files changed, 11 insertions(+), 52 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/AdminTests.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/AdminTests.cs index a925c4c9ce3..cbb80b4e769 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/AdminTests.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy.Tests/AdminTests.cs @@ -939,33 +939,7 @@ public async Task TestAddSanitizerContinuesWithTwoRequiredParams() } [Fact] - public async Task RemoveSanitizerErrorsForInvalidIdOnRecording() - { - RecordingHandler testRecordingHandler = new RecordingHandler(Directory.GetCurrentDirectory()); - var httpContext = new DefaultHttpContext(); - await testRecordingHandler.StartPlaybackAsync("Test.RecordEntries/oauth_request_with_variables.json", httpContext.Response); - var recordingId = httpContext.Response.Headers["x-recording-id"]; - httpContext.Request.Headers["x-recording-id"] = recordingId; - httpContext.Response.Body = new MemoryStream(); - var controller = new Admin(testRecordingHandler, _nullLogger) - { - ControllerContext = new ControllerContext() - { - HttpContext = httpContext - } - }; - - var testSet = new RemoveSanitizerList() { Sanitizers = new List() { "0" } }; - - var assertion = await Assert.ThrowsAsync( - async () => await controller.RemoveSanitizers(testSet) - ); - - Assert.Contains("Unable to remove 1 sanitizer. Detailed list follows: \nThe requested sanitizer for removal \"0\" is not active on recording/playback with id", assertion.Message); - } - - [Fact] - public async Task RemoveSanitizerErrorsForInvalidId() + public async Task RemoveSanitizersSilentlyAcceptsInvalidSanitizer() { RecordingHandler testRecordingHandler = new RecordingHandler(Directory.GetCurrentDirectory()); var httpContext = new DefaultHttpContext(); @@ -980,11 +954,9 @@ public async Task RemoveSanitizerErrorsForInvalidId() var testSet = new RemoveSanitizerList() { Sanitizers = new List() { "AZSDK00-1" } }; - var assertion = await Assert.ThrowsAsync( - async () => await controller.RemoveSanitizers(testSet) - ); + await controller.RemoveSanitizers(testSet); - Assert.Equal("Unable to remove 1 sanitizer. Detailed list follows: \nThe requested sanitizer for removal \"AZSDK00-1\" is not active at the session level.", assertion.Message); + Assert.Equal(200, httpContext.Response.StatusCode); } [Fact] diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Admin.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Admin.cs index 91f1610af61..60c5874a454 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Admin.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Admin.cs @@ -70,7 +70,6 @@ public async Task RemoveSanitizers([FromBody]RemoveSanitizerList sanitizerList) var recordingId = RecordingHandler.GetHeader(Request, "x-recording-id", allowNulls: true); var removedSanitizers = new List(); - var exceptionsList = new List(); if (sanitizerList.Sanitizers.Count == 0) { @@ -78,31 +77,19 @@ public async Task RemoveSanitizers([FromBody]RemoveSanitizerList sanitizerList) } foreach(var sanitizerId in sanitizerList.Sanitizers) { - try + var removedId = await _recordingHandler.UnregisterSanitizer(sanitizerId, recordingId); + if (!string.IsNullOrWhiteSpace(removedId)) { - var removedId = await _recordingHandler.UnregisterSanitizer(sanitizerId, recordingId); removedSanitizers.Add(sanitizerId); } - catch (HttpException ex) { - exceptionsList.Add(ex.Message); - } } - if (exceptionsList.Count > 0) - { - var varExceptionMessage = $"Unable to remove {exceptionsList.Count} sanitizer{(exceptionsList.Count > 1 ? 's' : string.Empty)}. Detailed list follows: \n" - + string.Join("\n", exceptionsList); - throw new HttpException(HttpStatusCode.BadRequest, varExceptionMessage); - } - else - { - var json = JsonSerializer.Serialize(new { Removed = removedSanitizers }); + var json = JsonSerializer.Serialize(new { Removed = removedSanitizers }); - Response.ContentType = "application/json"; - Response.ContentLength = json.Length; + Response.ContentType = "application/json"; + Response.ContentLength = json.Length; - await Response.WriteAsync(json); - } + await Response.WriteAsync(json); } [HttpGet] diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs index de5a49e5ddf..792ebce87c1 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs @@ -892,7 +892,7 @@ public async Task Unregister(string sanitizerId) SessionSanitizerLock.Release(); } - throw new HttpException(System.Net.HttpStatusCode.BadRequest, $"The requested sanitizer for removal \"{sanitizerId}\" is not active at the session level."); + return string.Empty; } /// @@ -918,7 +918,7 @@ public async Task Unregister(string sanitizerId, ModifiableRecordSession session.SanitizerLock.Release(); } - throw new HttpException(System.Net.HttpStatusCode.BadRequest, $"The requested sanitizer for removal \"{sanitizerId}\" is not active on recording/playback with id \"{session.SessionId}\"."); + return string.Empty; } ///