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
Azure · Aug 4, 2024 · 7f918db · 7f918db
1 parent ae5a7b3
commit 7f918db
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 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
@@ -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<string>() { "0" } };
-
-            var assertion = await Assert.ThrowsAsync<HttpException>(
-                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<string>() { "AZSDK00-1" } };
 
-            var assertion = await Assert.ThrowsAsync<HttpException>(
-                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
@@ -70,39 +70,26 @@ public async Task RemoveSanitizers([FromBody]RemoveSanitizerList sanitizerList)
             var recordingId = RecordingHandler.GetHeader(Request, "x-recording-id", allowNulls: true);
 
             var removedSanitizers = new List<string>();
-            var exceptionsList = new List<string>();
 
             if (sanitizerList.Sanitizers.Count == 0)
             {
                 throw new HttpException(HttpStatusCode.BadRequest, "At least one sanitizerId for removal must be provided.");
             }
 
             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
@@ -892,7 +892,7 @@ public async Task<string> 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;
         }
 
         /// <summary>
@@ -918,7 +918,7 @@ public async Task<string> 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;
         }
 
         /// <summary>